Validating input variables

Question

I have written the following JavaScript code and want to ask if it is written as per best practices. Please recommend any changes to be made in the code. The code is taking a few variables as input and then validating if all of them have some value and not blank/null.

var var1 = false;
var missingVAL = false;
var validationMsg = "";


host       = scriptletContext.get("host");
user    = scriptletContext.get("user");
serverID        = scriptletContext.get("serverId");
runID              = scriptletContext.get("runId");


if (host == false)
{
  validationMsg = "serverHostName (serverHostName) is missing.";
  var1 = true;
}

if (user == false)
{
    if (var1 == true)
    {
        validationMsg = validationMsg + " Requestor user name (requestorUserName) is missing.";
    }
    else
    {
        validationMsg = "Requestor user name (requestorUserName) is missing.";
    }
    var1 = true;
}

if (serverID == false)
{
    if (var1 == true)
    {
        validationMsg = validationMsg + " (vascoServerId) is missing.";
    }
    else
    {
        validationMsg = "(vascoServerId) is missing.";
    }
    var1 = true;
}

if (runID == false)
{
    if (var1 == true)
    {
        validationMsg = validationMsg + " OAC run type (runTypeId) is missing.";
    }
    else
    {
        validationMsg = "OAC run type (runTypeId) is missing.";
    }
    var1 = true;
}

if (var1 == true)
{
    scriptletContext.putGlobal("validationMsg", validationMsg);
}

scriptletContext.putGlobal("missingVAL", var1);

Answer 1

A quick note on style, it's much more common in JavaScript code to have the opening curly bracket on the same line as the expression that precedes it, rather than on a new line.

---

var1, based on the code, indicates whether there has been a validation problem, and to keep the same logical flow, we could name it instead isInvalid for more clarity.

Along the same lines, missingVAL is never used; perhaps you meant to use that one instead of var1?

---

It's a good practice to use strict equality/inequality (=== and !==) instead of == and != in many cases, to avoid some unpredictable behavior that is otherwise caused by the JavaScript engine.

For instance:

1 == "1"  //true
1 === "1" //false
null == undefined  //true
null === undefined //false
"" == 0  // true
"" === 0 //false

Also, when comparing values to true or false, it's simpler to just let the truthiness or falseness of the value be the condition, such as:

// instead of this:
if (someValue == true && someOtherValue == false) { ... }
// simply do this: (note the use of not "!" operator)
if (someValue && !someOtherValue) { ... }

---

You are using the same validation routine 4 separate times; instead of copying the code, it's simpler to make one function that does the work then just call it 4 times.

Your validations do this:

1. Check that the input value is not false (empty, undefined or null)

2. If it fails, add a message into a string noting the validation failed

3. Make the validation flag variable true to indicate there is a validation problem.

Step (3) could reasonably be removed by using the validationMsg, since if there have been no validation failures by the end, it will still be an empty string.

We could write a function like this: (code comments added for explanation)

function validate(value, messageIfInvalid) {
    // null, undefined, 0 and "" evaluate to false
    if (!value) {
        // empty string evaluates to false and non-empty to true
        if (validationMsg) {
            // if not empty, add a space at the end
            validationMsg += " ";
        }
        // add the messageIfInvalid
        validationMsg += messageIfInvalid;
    }
}

This function introduces a side effect which is to modify the globally-scoped variable validationMsg, which makes it less predictable. It would be generally better to instead pass it the validationMsg as argument and then return the validationMsg with any applicable modifications made by the function, like so:

// also pass validationMsg as argument
function validate(value, validationMsg, messageIfInvalid) {
    if (!value) {
        if (validationMsg) {
            validationMsg += " ";
        }
        validationMsg += messageIfInvalid;
    }
    // return the validationMsg, whether or not it was modified
    return validationMsg;
}

Then at the very end of the script, we can check whether validationMsg is an empty string or not to find out if there have been any invalid inputs.

// this condition will be true unless validationMsg is an empty string
if (validationMsg) {
    scriptletContext.putGlobal("validationMsg", validationMsg);
    scriptletContext.putGlobal("missingVAL", true);
} else {
    scriptletContext.putGlobal("missingVAL", false);
}

---

There is one potential issue with this logic, in that if a value checked as invalid/false, but the messageIfInvalid argument was passed an empty string, then no message would be added to validationMsg. We can add another condition to address this special case and just add a generic validation message, like so:

if (!value) {
    if (!messageIfInvalid) {
        messageIfInvalid = "a value is missing";
    }
    if (validationMsg) {
        validationMsg += " ";
    }
    validationMsg += messageIfInvalid;
}

---

This is what it looks like with all the above applied.

host = scriptletContext.get("host");
user = scriptletContext.get("user");
serverId = scriptletContext.get("serverId");
runId = scriptletContext.get("runId");

function validate(value, validationMsg, messageIfInvalid) {
    if (!value) {
        if (!messageIfInvalid) {
            messageIfInvalid = "a value is missing";
        }
        if (validationMsg) {
            validationMsg += " ";
        }
        validationMsg += messageIfInvalid;
    }
    return validationMsg;
}

var validationMsg = "";

validationMessage = validate(host, validationMessage, "serverHostName (serverHostName) is missing.");
validationMessage = validate(user, validationMessage, "Requestor user name (requestorUserName) is missing.");
validationMessage = validate(serverId, validationMessage, "(vascoServerId) is missing.");
validationMessage = validate(runId, validationMessage, "OAC run type (runTypeId) is missing.");

if (validationMsg) {
    scriptletContext.putGlobal("validationMsg", validationMsg);
    scriptletContext.putGlobal("missingVAL", true);
} else {
    scriptletContext.putGlobal("missingVAL", false);
}