Wednesday, March 18, 2009

On using "if(foo)" vs "if(foo != null)"

I was going to leave this topic for a planned series of posts detailing (and explaining) my personal coding style, as it differs somewhat to that laid out by Adobe. But, a few people noticed a comment I made about the subject, so I thought I'd rant on the subject here, where I can explain myself.

Adobe's guidelines tell us that to test if foo is not null, we should use something like the following:

if (foo)
    doSomething();

This does work, and I find it makes for nice readable code. My coding style is based around readability and "pretty", so I do this all the time.

However, there are a lot of places where we're forced by ActionScript's limited overloading capabilities to declare method parameters as Object or *. And this is where bugs creep in.

Let's say you have a method like so:

public function doStuf(howMany : Number, usefulObject : MyClass = null) : void
{
    //Do some stuff

    if (usefulObject)
        doOtherStuff(usefulObject);
}

As it is, this will work fine. You have a howMany argument, and an optional usefulObject argument.

Everything is going fine, and one day you decide that you can actually use the doStuff() function on all sorts of different objects, as well as instances of MyClass - but you'd like to keep the semantics of it the same, so the parameter is still optional.

You've just introduced a bug. One that you won't trip up any existing unit tests, too.

If you're lucky you'll find it soon, but probably not. If it's a week or two before you need to pass in an empty string, or the number 0, or false, you'll be left scratching your head wondering why it's not doing what it's supposed to.

This is why I think it's not necessarily a good practice to be recommending to ActionScript beginners.

I still do it, but I'm trying to weed it out. Once I break the habit, it'll definitely cut down on the syntax errors I put in my Java, anyway ;-)

3 comments:

senocular said...

I think another thing to consider are E4X XMLList objects. When empty, they have a value of undefined, though their Boolean conversion (used in if statements) still returns true. So:

trace(Boolean(new XMLList())); // true
trace(new XMLList() == undefined); // true

They can both exist or not exist depending on how you write your condition.

Stas said...

I think "if (myInstance)" should be changed to "if (usefulObject)"?

Josh McDonald said...

@Stas - you're right, cheers!

This is

  • Tales of Flex
  • From Brisbane, Australia
  • Opinions on Flex development
  • Tips and FAQs
  • Shameless self-promotion

I am

  • Twitterer
  • Flexcoder
  • Maroon
  • Designer
  • Java lover
  • That loud-mouthed Aussie yob
  • Blogger
  • Problem solver
  • Contributor
  • Cricket Fan
  • Lousy photographer
  • Great cook

I read



Subscribe via RSS to receive updates!