In some javascript code I’m working on refactoring I’ve handled cases where I wanted to default an object property to true without having to go through the code-base and add the property to every instance of the object by doing something like this:
if(typeof(thisBool) === 'undefined' || thisBool) {
//do cool stuff here;
}
so for existing instances I only had to fix up the model in cases where I wanted to set thisBool to some value calculated somewhere else, or to set it to false; if it was unset then this code treated it as true.
The objects are built dynamically when used – they’re not defined classes, but are “instantiated” something like this:
var thing1 = {
Name: 'thing No 1',
HasCat: false,
[thisBool: true]
};
and thing1 is passed into the function
Is this a terrible idea? I’m updating that code (and all of the instances) so now is the time to fix this, if it’s a code smell.
For my own part I would rather be more explicit, but I also see the value in not having to supply a thisBool: true in hundreds of places, where by default thisBool should be true. Is there a better way to achieve this?
0
Assuming this is a function argument there is no need to use typeof as the variable will always exist. The easiest and cleanest way to make it default to true is this:
if(thisBool === undefined || thisBool)
Of course that could be done a bit more explicit:
if(thisBool === undefined) {
thisBool = true;
}
if(thisBool) { ... }
8
I don’t think there is anything terribly wrong with the if statements provided. It is not a big deal but I am not a fan of having a method outside of that instantiated object determining the defaults for that object. I don’t know the codebase so I could’t tell you if it is worth refactoring.
If you are writing something from scratch I would suggest using inheritance to define default properties such as bools strings and numbers.
function Thing(config) {
for(key in config) {
this[key] = config[key];
}
}
Thing.prototype.bool = true;
new Thing().bool
>> true
new Thing({bool: false}).bool
>>false
Also it would be quite easy to integrate the Thing object in the existing code base. Instead of :
var thing1 = {
Name: 'thing No 1',
HasCat: false,
[thisBool: true]
};
It would be:
var thing1 = new Thing({
Name: 'thing No 1',
HasCat: false
});
thing objects should be responsible for setting their own default values. If you force other functions to know that the default for thisBool is true, your code becomes harder to understand. Also, you run the risk that someone might add a new function but not know that thisBool is supposed to be true by default.
Below is my suggestion for a constructor-like function.
var createThing = function () {
var myThing = {};
myThing.thisBool = true;
return myThing;
};
var thing1 = createThing();
thing1.name = "thing no 1";
...
On a pragmatic note, it might not be feasible for you to make the changes described above. If you have thousands of thing objects, a couple of functions, and a lack of justification to change all of the thing objects, you may need another solution.
One option is for you to change the semantics of thisBool so that the default is false. Because properties are undefined by default and undefined is falsey, you would reduce the risk of surprising future programmers.
if (notThisBool) { // hopefully you have better semantics than notThisBool
// do special case
} else {
// do default case
}
As a last resort, I would suggest that your function check for exactly false, and include a comment explaining why.
// check for exactly false because thisBool is an optional property
// that should be considered true unless it is set to false
if (myThing.thisBool === false) {
// special case
} else {
// default case
}
I realize that “not exactly false” is slightly different than “undefined or truthy”. (In my above code a falsey value other than “false” would run the default case.) I think that my code is more clear.