# Using large amounts of if-else statements for playing card numbers

I have a method that uses a bunch of if-else statements, and I am thinking how I could simplify it.

public static CardNumber decode(String s) {
if(s == null) {
return null;
} else if(s.equalsIgnoreCase(ACE.toString())) {
return ACE;
} else if(s.equalsIgnoreCase(TWO.toString())) {
return TWO;
} else if(s.equalsIgnoreCase(THREE.toString())) {
return THREE;
} else if(s.equalsIgnoreCase(FOUR.toString())) {
return FOUR;
} else if(s.equalsIgnoreCase(FIVE.toString())) {
return FIVE;
} else if(s.equalsIgnoreCase(SIX.toString())) {
return SIX;
} else if(s.equalsIgnoreCase(SEVEN.toString())) {
return SEVEN;
} else if(s.equalsIgnoreCase(EIGHT.toString())) {
return EIGHT;
} else if(s.equalsIgnoreCase(NINE.toString())) {
return NINE;
} else if(s.equalsIgnoreCase(TEN.toString())) {
return TEN;
} else if(s.equalsIgnoreCase(JACK.toString())) {
return JACK;
} else if(s.equalsIgnoreCase(QUEEN.toString())) {
return QUEEN;
} else if(s.equalsIgnoreCase(KING.toString())) {
return KING;
} else {
return null;
}
}


The whole class is here:

public class CardNumber {
private final String name;
private final int value;

public static final CardNumber ACE_AS_ONE = new CardNumber("A", 1);
public static final CardNumber TWO = new CardNumber("2", 2);
public static final CardNumber THREE = new CardNumber("3", 3);
public static final CardNumber FOUR = new CardNumber("4", 4);
public static final CardNumber FIVE = new CardNumber("5", 5);
public static final CardNumber SIX = new CardNumber("6", 6);
public static final CardNumber SEVEN = new CardNumber("7", 7);
public static final CardNumber EIGHT = new CardNumber("8", 8);
public static final CardNumber NINE = new CardNumber("9", 9);
public static final CardNumber TEN = new CardNumber("10", 10);
public static final CardNumber JACK = new CardNumber("J", 11);
public static final CardNumber QUEEN = new CardNumber("Q", 12);
public static final CardNumber KING = new CardNumber("K", 13);
public static final CardNumber ACE = new CardNumber("A", 14);

private CardNumber(String name, int value) {
this.name = name;
this.value = value;
}

@Override
public String toString() {
return name;
}

public static CardNumber decode(String s) {
if(s == null) {
return null;
} else if(s.equalsIgnoreCase(ACE.toString())) {
return ACE;
} else if(s.equalsIgnoreCase(TWO.toString())) {
return TWO;
} else if(s.equalsIgnoreCase(THREE.toString())) {
return THREE;
} else if(s.equalsIgnoreCase(FOUR.toString())) {
return FOUR;
} else if(s.equalsIgnoreCase(FIVE.toString())) {
return FIVE;
} else if(s.equalsIgnoreCase(SIX.toString())) {
return SIX;
} else if(s.equalsIgnoreCase(SEVEN.toString())) {
return SEVEN;
} else if(s.equalsIgnoreCase(EIGHT.toString())) {
return EIGHT;
} else if(s.equalsIgnoreCase(NINE.toString())) {
return NINE;
} else if(s.equalsIgnoreCase(TEN.toString())) {
return TEN;
} else if(s.equalsIgnoreCase(JACK.toString())) {
return JACK;
} else if(s.equalsIgnoreCase(QUEEN.toString())) {
return QUEEN;
} else if(s.equalsIgnoreCase(KING.toString())) {
return KING;
} else {
return null;
}
}

public int getValue() {
return value;
}
}

-

If you still want to have all the power of a class instead of an enum, another solution would be to use a map:

public class CardNumber {
private final String name;
private final int value;

private static final Map<String, CardNumber> cards = new HashMap<>(14);

static {
cards.put("A", new CardNumber("A", 1));
//[...]
cards.put("K", new CardNumber("K", 13));
}

// [... constructor ...]

public static CardNumber decode(String s) {
if(s == null) {
return null;
} else {
return cards.get(s);
}
}

// [... toString, getValue ...]
}

-
With a private constructor there's no need for a class. What's this 'power of a class' that you speak of? – Simon Forsberg Sep 1 '14 at 19:36
@SimonAndréForsberg I overlooked the private keyword. And I'm talking about extending, etc. – tim Sep 1 '14 at 19:41
OK, extending I can agree with. Although you don't get much use of extending if the superclass has a private constructor. What's the 'etc.' part? :) – Simon Forsberg Sep 1 '14 at 20:42
Well, you can't create new instances of them. So for example if we have two sets of cards (which is not that uncommon), and the cards have a state (for example open on the table or not), that would not be possible with an enum. Although I get your point, that would require a restructure of the class anyways. – tim Sep 1 '14 at 21:08
Additionally, with an enum, you'd get a hashCode + equals implementation for free! Not that you need one right now because essentially, this class has been written as if it was an enum already... – Simon Forsberg Sep 1 '14 at 21:37

Convert your constant values to an enum instead, for example:

enum Card {
ACE_AS_ONE("A", 1),
TWO("2", 2),
THREE("3", 3),
// and so on
KING("K", 10);

private final String name;
private final int value;

Card(String name, int value) {
this.name = name;
this.value = value;
}

public static Card decode(String name) {
for (Card card : Card.values()) {
if (card.name.equalsIgnoreCase(name)) {
return card;
}
}
return null;
}
}


While at it, add some unit tests to verify, for example:

public class CardTest {
@Test
public void testDecodeNumeric() {
assertEquals(Card.TWO, Card.decode("2"));
assertEquals(Card.THREE, Card.decode("3"));
}

@Test
public void testDecodeLabeled() {
assertEquals(Card.KING, Card.decode("K"));
}

@Test
public void testNonexistent() {
assertNull(Card.decode("nonexistent"));
}

@Test
public void testDecodeNull() {
assertNull(Card.decode(null));
}
}

-
+1 for ACE_AS_ONE, given that in some card games 'Ace' can optionally represent the biggest value in a suit as well. – h.j.k. Sep 2 '14 at 3:53
Thanks @h.j.k. but ACE_AS_ONE was the OP's idea, not mine ;-) – janos Sep 2 '14 at 6:32
After a more detailed review, that means OP's code is buggy too... "A" will always give you CardNumber("A", 14). Oh wells... – h.j.k. Sep 2 '14 at 6:56
Yes, he can't get Card.ACE_AS_ONE through .decode, but we don't know the rest of his code and it might be acceptable for him, who knows. – janos Sep 2 '14 at 7:27
I'd drop value in favor of ordinal()+1. This may stop working if the requirements change, but they won't (and simpler code now is better than some theoretical flexibility). – maaartinus Sep 3 '14 at 0:22