First a little bit of context (you can skip this part if you prefer to focus on code).
I joined a new project that will be integrated in a nodeJS's platform. Our team does have experience with JEE enterprise Web Apps. That sets up our background. This new project will consume REST APIs, aggregate some data, implement some business logic and pass these data to front-end consumer. Sort of lightweight microservice architecture.
Some co-workers started to work on the project and I found it that in the source code we do have a lot of code snippet like if (foo != null && foo.property != null) {return foo.property.value;}
Foo being supposed to be an object passed as an argument to a function which would implement that kind of test.
A snippet example will talk more.
Let's pretend that's the response of an API i am consuming. I want to write a small function which would return statusValue if the object does exist, if it's not null or undefined, not empty and the property does exist and isn't blank or empty for instance.
var response = {
"services":[],
"metadata": {
"status": "statusValue"
}
};
That's how it is as for now :
function getStatusValue(response) {
if (response != null && response.metadata != null) {
return response.metadata.status;
}
};
What would be considered as a best JS practise to implement that (we are also using lodash so maybe it's a better option to use lodash internals for that). I am just fearing we are transposing our Java habbits.
Basically i would like to know how to check for null, undefined, empty, blank safely (if that makes sense). What would be the JS best practice for that in 2016 with libraries such as lodash and stuff.
When checking for properties of an object, checking for loose equality to null is not enough.
Loose equality (==) employs type hinting in JavaScript, which attempts converting the members to a common type that can then be used for determining whether they are equal or not.
As such, best practices for JavaScript dictate that strict equality (===) is always to be used, in order to avoid edge-case scenarios or unknown behavior when checking for values or types.
You can find more info about loose vs strict equality here:
While this is not a must-have in your function, it is a good practice to follow and develop as a habit, so that on later code the implications of loose equality (==) could be avoided (e.g. avoiding '3' == 3, when a number or string type is expected).
Although considered a downside of the language by some, using null is similar in intent as checking for undefined, but is actually meant to express in the code that the coder is expecting an object (or lack of) to be provided, instead of a primitive type (number, string, boolean, function). This differs from Java, or any other typed language, where null is used for Object type defined variables; but in JavaScript there's no constraining a variable to given type.
You can find out more details about null at MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/null
In practice, and especially when dealing with values that come outside of a unit - which is the case of any API - is considered a best practice to actually check for the type being an object, in order to make sure it has properties.
While your code is not incorrect, it does show that there's not much attention to best practices, which eventually will lead to writing buggy code.
My suggestion for your code, following best practices and independent of any library, is:
function getStatusValue(response) {
// must be of type `object`, to have properties.
if (typeof response === 'object' &&
// any variable of type `object` might be `null`, so exclude that.
response !== null &&
// `metadata` property must be of type `object`, to have properties.
typeof response.metadata !== 'object' &&
// `metadata` is of type `object`, check for not being `null`.
response.metadata !== null) {
// `status` property is expected to be a string, given by the API spec
if (typeof response.metadata.status === 'string' &&
// on strings, we can access the `length` property to check if empty string (0 chars).
response.metadata.status.length > 0) {
// all good, return `status` property.
return response.metadata.status;
} else {
// `status` property is either not a string, or an empty string ('').
}
} else {
// invalid `response` object.
}
}
Also, it might be easier to create or use from any libraries you integrate, a function to check for a valid object; something like _.isObject or this:
function isObject(o) {
return (typeof o === 'object' && o !== null);
}
which you could later be used in the above snipped like so:
function getStatusValue(response) {
if (isObject(response) && isObject(response.metadata)) {
if (typeof response.metadata.status === 'string' &&
response.metadata.status.length > 0) {
return response.metadata.status;
} else {
// `status` property is either not a string, or an empty string ('').
}
} else {
// invalid `response` object.
}
}
As a final thought, as it is clearly visible, following best practices does increase the size of your code, but the benefit is that the resulting code is much safer, with less chances of throwing exceptions (root of many app crashes), easier to collaborate on, and easier to build tests/coverage for.