Namek Dev
a developer's log
NamekDev

Don't call it empty, make it empty

March 19, 2014

Context: game project.

class Card {
  var id:String;

  // ... some more fields and ctor setting the `id` field
}

class EmptyCard extends Card {
  public EmptyCard() {
    id = "";
    // and so on
  }
}

True story. Let’s refactor that.

Benefits?

Yeah, there is one.

var card:Card = getSomeCardFromServerOrSomething();
if (card is EmptyCard) {
  // ...
}

That condition just looks nice.

OK, I don’t get what’s wrong here

Imagine situation when there are some cards which don’t have backgroundImageName because they’re deserialized from some server JSON data.  When this field is empty we need to fill it:

var card:Card = getSomeCardFromServerOrSomething();
if (card.backgroundImageName == null || card.imgName == "") {
  card.backgroundImageName = CardUtils.getImgNameForFaction(card.faction);
}

In such cases we may miss that our card is of type EmptyCard. As we continue we try to fill it with missing_ data_ based onmissing data (there is no faction inside). Two failures at once!

Also that comparison and many checks of values for certain fields - is it null or is it empty String? For me it’s a third failure.

Alternative

Alternative is to not use Card object with empty fields. It’s better to just give null instead of whole empty object. Doing so you will be sure that no one fills EmptyCard object (got as Card reference) by mistake (as in last code snippet).

And if you really, really want to check whether object is not being filled, it’s better to use some method for it. It’s always based on context but here still I don’t recommend it.

class Card {
  function isCardEmpty():Boolean {
    return id == null; //or whatever
  }
}

Conclusion

You should consider yourself what you want to tell other programmers. But to me using such EmptyCard classes is just hiding some important things for new developers who may not notice such “important” buggy class which does not have any role in OOP and its use does not seem to be coherent in any way with logic layer.

And hey! Does it look that bad?

var card:Card = getSomeCardFromServerOrSomething();
if (card == null) {
  // ...
}
gamedev, refactoring
comments powered by Disqus