getCharacter variants

[MrHen]

All of the various getCharacter variants are starting to bug me and I want to collapse things into a common pattern. Which of these is preferred?

public Minion getCurrentPlayerCharacter(int index) {
    return getCharacter(PlayerSide.CURRENT_PLAYER, index);
}

public Minion getCharacter(PlayerSide playerSide, int index) {
    PlayerModel playerModel = modelForSide(playerSide);
    return index == 0 ? playerModel.getHero() : playerModel.getMinions().get(index - 1);
}

Example usage:

Minion cTargetMinion = cNode.data_.getCurrentPlayerCharacter(targetCharacterIndex);

Minion cTargetMinion = cNode.data_.getCharacter(PlayerSide.CURRENT_PLAYER, targetCharacterIndex);

I don't have a strong preference other than wanting only one.

[oyachai]

No strong feelings, but I'd vote for keeping the more generic version, i.e., getCharacter(). It's not that much more verbose than the other one.

[Kallin]

I don't mind having both, with getCurrentPlayerCharacter being a wrapper around getCharacter. It makes future refactorings easier. Then, the only time you need to use getCharacter is when you're handed a PlayerSide parameter and don't know if it's the current or target.

[MrHen]

@Kallin I can understand that, I guess. I just find the current pattern rather annoying since there are a whole bunch of getCurrent/WaitingThing(x) and, conceptually, it isn't any harder to deal with getThing(Player, x). It isn't terribly hard to refactor either way.

But your point about not knowing which PlayerSide you've got is a good argument for at least keeping getThing(Player, x). We shouldn't remove the generic version.

[MrHen]

Well, I committed a deprecate pass on my fork if anyone is curious about what it looks like.

75924a5e80231e55c2873566238eeb516160cfb2

I guess I can live without this change if @Kallin prefers it the old way so feel free to never take this commit. I'm moving on to other issues, though, so I'll close this in a few days if no one has anything else to say.

[oyachai]

I think it's largely a style issue, so as long as it doesn't annoy anyone too much, I'm inclined to keep the old way for now.

[MrHen]

Sounds good.