Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is redundant code acceptable if it improves readability?

I have the following code to check if a game unit is a player or an enemy. These are the only two categories. I could delete the isEnemy method and run all checks for enemy as if(!isPlayer), but I personally feel that if(isEnemy) makes the intent of the code clearer. Are there any established coding styles that have anything to say about this kind of situation?

public boolean isPlayer(Unit unit) {
    return unit == player;
}

public boolean isEnemy(Unit unit) {
    for (Unit e : enemies) {
        if (unit.equals(e))
            return true;
    }
    return false;
}
like image 901
talloaktrees Avatar asked Dec 25 '12 03:12

talloaktrees


2 Answers

For your case, you have only two possible states - they're either an enemy, or they're a player. If they're a player, they are not an enemy. The cleanest way to express that would be !isPlayer.

If you had other possible states,then you may want to look into some sort of enumeration on the other states.

As a general rule of thumb: Don't Repeat Yourself. If one part of your code changes that you've duplicated (perhaps to fix a bug), you then have to change every occurrence of that bug. It can turn into a maintenance nightmare.

like image 179
Makoto Avatar answered Nov 15 '22 03:11

Makoto


Personally, I would implement isEnemy() as !isPlayer() but have the an assert that runs the loop to make sure it really is an enemy.

like image 22
steveha Avatar answered Nov 15 '22 05:11

steveha