Cannot decide: guava or plain old for loop

gistfile1.java

public Boolean hasStraightFlush_plainJava() {
    Suit expectedSuit = cards.get(0).getSuit();
    int minValue = cards.get(0).getValue().ordinal();
    for (int i = 1; i < 5; i++) {
        Card card = cards.get(i);
        if (card.getValue().ordinal() != minValue + i) {
            return false;
        }
        if (card.getSuit() != expectedSuit) {
            return false;
        }
    }
    return true;
}

public Boolean hasStraightFlush_guava() {
    Suit expectedSuit = cards.get(0).getSuit();
    int minValue = cards.get(0).getValue().ordinal();
    ArrayList<String> expectedFlush = Lists.newArrayList();
    for (int i = 0; i < 5; i++) {
        expectedFlush.add((minValue + i) + ":" + expectedSuit);
    }
    String expected = on(',').join(expectedFlush);
    String actual = on(',').join(transform(cards, new Function<Card, String>() {
        public String apply(Card input) {
            return input.getValue().ordinal() + ":" + input.getSuit().name();
        }
    }));
    return expected.equals(actual);
}

I'm sure the guava implementation could be improved...

What you expected is to compare two hands. If you have a well defined equals() and hashCode() in Card, you can do it this way:

return Iterables.all(expectedCards, Predicates.in(cards)); // cards is made of exactly 5 cards

Where expectedCards is built like this:

List<Cards> expectedCards = buildStraightFlushFrom(Iterables.getFirst(cards));

With

public static List<Card> buildStraightFlushFrom(Card card) {
    List<Cards> result = Lists.newArrayList();
    Suit suit = card.getSuit();
    int minValue = card.getValue().ordinal();
    for (int i = 0; i < 5; i++) {
        result.add(Card.get(suit, Value.values()[minValue + i]));
    }
    return result;
}

It could be better. But there you'll need an operator like zip() at least, and it isn't present in Guava.

How about this:

public Boolean hasStraightFlush_guava() {
    Iterable<Card> cardsExceptFirst = Iterables.skip(cards, 1);
    Iterable<Card> cardsInSameSuit = Iterables.filter(cardsExceptFirst,
        new InSameSuitAs(cards.get(0).getSuit()));
    Iterable<Card> cardsInSequence = Iterables.filter(cardsInSameSuit,
        new InSequenceAfter(cards.get(0)));
    return Iterables.size(cardsInSequence) + 1 == 5;
}

private static final class InSameSuitAs implements Predicate<Card> {
    private final Suit suit;

    public InSameSuitAs(Suit suit) {
        this.suit = suit;
    }

    @Override
    public boolean apply(Card input) {
        return input.getSuit() != suit;
    }
}

private static final class InSequenceAfter implements Predicate<Card> {
    private Card previousCard;

    private InSequenceAfter(Card previousCard) {
        this.previousCard = previousCard;
    }

    @Override
    public boolean apply(Card input) {
        boolean inSequenceAfterPreviousCard = input.getValue().ordinal() == previousCard
            .getValue().ordinal() + 1;
        previousCard = input;
        return inSequenceAfterPreviousCard;
    }
}

I didn't write unit tests, so I'm not 100% sure it will work. Also, the code is a little verbose, but I do feel that this is necessary when using Predicates.

How would you write it in a "natural langage" ?

"a sorted hand is a suite when all of the cards, except first one, is the successor of the previous" ?

And how would you do it in another langage ?
Say ruby ?

loop and early return

irb(main):025:0> def flush? hand; 1.upto(4) { |i| return false unless hand[i] == hand[i-1] + 1 }; true; end
=> nil
irb(main):026:0> flush? [1,2,3,4,6]
=> false
irb(main):027:0> flush? [1,2,3,4,5]
=> true

reduce and early return

irb(main):039:0> def flush? hand; head, *tail = hand; !!tail.reduce(head) { |previous, card| return false unless card == previous + 1; card}; end
=> nil
irb(main):040:0> flush? [1,2,3,4,5]
=> true
irb(main):041:0> flush? [1,2,3,4,6]
=> false

A loop is enough ?

(might look later in erl and java...)

I have another proposal built on the all (Enumerable#all?)

And all predicate is given each element in the list, until predicate is false ...
It depends on the struct at hand, and if my list is [1,2,3,4,5] then I'm toasted ! I have to build a list of element that carries a previous

erl

So given L = [4,5,6,7,8], I build a list of pairs {Previous, Current} for the tail list ([5,6,7,8]), then apply all with predicate : Previous + 1 == Current

Now, that is allocating 2 lists in the course (seq, comprehension) ... so I dislike it

19> Flush = fun(L) -> lists:all(fun({Previous,Current}) -> Previous+1==Current end, [ {lists:nth(N-1, L), lists:nth(N, L)} || N <- lists:seq(2, length(L)) ]) end.
#Fun<erl_eval.6.13229925>
20> Flush([5,6,7,9]).                                                                                                           
false                           
21> Flush([5,6,7,8]).
true

Here is a function that does not allocate more lists

flush(L) ->
  [Head | Tail] = L,
  flush(Head, Tail).

flush(_, []) ->
  true;
flush(Previous, [Current | Tail]) ->
  (Previous + 1 == Current) and flush(Current, Tail).

:)
WDYT?