Since all the cards now are removable, there is no reason for this method whatsoever, since the only thing it offers is a set object with all the elements on the sequence, which can currently be accessed by the iterator.

Basically, every implementation of the CardSequenceType interface should ensure that it always has a valid state - that is, two cards are sequential to one another (depending on what being sequential means to that specific sequence type).

Thus, the function will never output a false value because the CardSequenceBuilder and the addCard, removeCard, split methods should never leave the sequence in an invalid state.

Now, the CardSequenceBuilder may leave the sequence on an unstable state, but not on an invalid one. The difference is that one means the sequence disregards its type validation - which is never to be allowed - and the other means that the cards are sequential to one another but cannot be validated fully. Implementation-wise, a card sequence is unstable when it has less than three cards.

The solution may be remove the hasValidState from the interface and its implementations (since they won't be called either way), and its occurrence - currently only in the isStable method.

The main issue here is that an invalid card sequence can be created, which is undesired for a robust Java application. The most elegant solution would be to use the Builder Design Pattern, which will allow the creation of a Card Sequence only if it meets the minimal requirements for a valid Card Sequence. The Builder will reside inside the Card Sequence abstract class and its methods will call abstract methods since the building process of a Card Sequence may vary from type to type. These are the Builder methods:

public Builder addListener(CardSequenceListener);
public Builder addCard(Card);
public CardSequence build() throws NotEnoughCardsException;

Some decisions can be implied by this interface:

• a Card Sequence does not need any Listeners whatsoever
• a Card Sequence can have multiple Listeners
• the user will only know if the Card Sequence being built is valid until the build method is called

One thing that might not be obvious to sight at first glance is the possibility of duplicate Listeners. This can be solved by using a Set instead of a List, so whenever adding a new Listener, a duplicate will not be considered. Implications of this change are that:

• when calling a listener method, the Class has to iterate through all the set and calling the method through each Listener.
• when creating a new sequence within the Class (e.g. when splitting a sequence), the whole Set has to be inherited
• a listener cannot be removed since it would imply that some other class removes a listener and the other, expecting it to be called, won't.

Also, since the whole point of having a Builder is to prohibit inappropriate card sequence initialization, the Card Sequence constructor will be private, meaning that it can only be instantiated through the Builder (which resides within the class). This point implies that, yes, the automatic tests will have to change a little, plus the Builder itself will have to be tested.

After reading this article by Robert Brautigam on DZone, I was convinced to get rid of null checks in the code of Rummy. The article shows that these checks make code less readable and null parameters acceptable. Instead, we should deal with null as a programming error and let NullPointerException occur.