Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question

3 Answers 3

up vote 8 down vote accepted

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) { ... }
share|improve this answer
8  
thisBool = thisbool === undefined ? true : thisBool;, just to make it all fancylike :-P –  Neal Jan 4 '13 at 17:48
    
Actually, thisBool is a property on a dynamically created (i.e. not defined as a class) object, and it may or may not exist on the object. Since there's no class I can't use prototypes to default it to true; to be explicit I have to go through the system and find all the "instances" that are passed to the function in question and add thisBool. Question updated to clarify. –  cori Jan 4 '13 at 18:13
    
If it's an object property you also don't need typeof. The only case where it's necessary is when dealing with plain variables - if they are not defined (not even === undefined) accessing them without typeof would throw an exception –  ThiefMaster Jan 4 '13 at 18:58
3  
beware that undefined can be redefined... –  ratchet freak Jan 4 '13 at 19:03
    
Ah that makes sense, thanks! –  cori Jan 4 '13 at 19:05

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
});
share|improve this answer

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.