Return to Answer

Guess who has a flash back?

###Smaller things:###

• Deck: I can't see any declaration of that type.
• Deck.shuffle(): Is only called within the implementation, so no need to make that public.
• AbstractPlayer: is not abstract
• AbstractPlayer: The hand is not private (it is implementation of abstraction, and should not be accessible to implementation of the abstraction... if that makes any sense)
• The Card itself should be displayed by the Prompter, too (think of adding a GUI)
• CardDeck: The Constructor has code duplication. The upper one should call the lower one with 1 as parameter. So initializeMultipleDecks will be initializeDecks, and initializeDeck can be removed. Then, you also get rid of the duplicate shuffle() call.
• CardDeck: Not sure if you have to pass the Suit[] and Rank[] array for the initializer methods. I think it's okay, to get the values from the enum itself. (If you want to enhance it later: Add the method with the two parameters again, and delegate the call from the no-param method to the newly added one).
• CardDeck.dealCard: There's a 'isEmpty' method in List. And: I can't find the place, where you actually check, if there's no card left? So, you will always end with an exception.
• CardDeck.dealCard: return deck.remove(0) does the job.
• Player.performAction: The method name doesn't tell me, what the method should do - but I see, this one is a bit tricky.
• Player.performAction: I think the loop can be simplified: Can't the if condition moved into the while condition? Also: You have an empty default case.
• Prompter.getState: State is not clear. The "H" and "S" is duplicated, that should be a constant with a proper name - maybe even provided in the enum itself (String symbol as constructor of the enum?). Also: the default case will return null. Since the code won't ever reach that point, I'd throw an Exception - if that happens, a bug has been introduced.
• Prompter.ask: Who asks what whom why?
• Rank: Can't remember, if I have ever seen a non-private constructor in an enum and I didn't know you can actually do that. However, it doesn't make any sense, since noone else can call it anyway?

Inheritance

A main object oriented principle is "lose coupling", inheritance can be, as I think it is in your case, the opposite. The concrete implementations are tightly coupled to its super class. My main problem with that, you can neither test the abstraction without a implementation, nor can you test a implementation without its abstraction.

Further, you're violating Liskov's substitution law (The L in SOLID): If your program works with type T, it should work with U extends T interchangeably, without changing your application. I think it's enough to explicitly declare a Dealer and a Player in that case. I hope someone corrects me, if I'm wrong, or describes it better: I always had a hard time understand the L and I usually have other reasons to avoid inheritance.

Another general rule is composition over inheritance. Meaning: Do not use inheritance for code reuse. It's much more flexible, easier to maintain and what not.

Hope that helps...

Guess who has a flash back?

Smaller things:

• Deck: I can't see any declaration of that type.
• Deck.shuffle(): Is only called within the implementation, so no need to make that public.
• AbstractPlayer: is not abstract
• AbstractPlayer: The hand is not private (it is implementation of abstraction, and should not be accessible to implementation of the abstraction... if that makes any sense)
• The Card itself should be displayed by the Prompter, too (think of adding a GUI)
• CardDeck: The Constructor has code duplication. The upper one should call the lower one with 1 as parameter. So initializeMultipleDecks will be initializeDecks, and initializeDeck can be removed. Then, you also get rid of the duplicate shuffle() call.
• CardDeck: Not sure if you have to pass the Suit[] and Rank[] array for the initializer methods. I think it's okay, to get the values from the enum itself. (If you want to enhance it later: Add the method with the two parameters again, and delegate the call from the no-param method to the newly added one).
• CardDeck.dealCard: There's a 'isEmpty' method in List. And: I can't find the place, where you actually check, if there's no card left? So, you will always end with an exception.
• CardDeck.dealCard: return deck.remove(0) does the job.
• Player.performAction: The method name doesn't tell me, what the method should do - but I see, this one is a bit tricky.
• Player.performAction: I think the loop can be simplified: Can't the if condition moved into the while condition? Also: You have an empty default case.
• Prompter.getState: State is not clear. The "H" and "S" is duplicated, that should be a constant with a proper name - maybe even provided in the enum itself (String symbol as constructor of the enum?). Also: the default case will return null. Since the code won't ever reach that point, I'd throw an Exception - if that happens, a bug has been introduced.
• Prompter.ask: Who asks what whom why?
• Rank: Can't remember, if I have ever seen a non-private constructor in an enum and I didn't know you can actually do that. However, it doesn't make any sense, since noone else can call it anyway?

Inheritance

A main object oriented principle is "lose coupling", inheritance can be, as I think it is in your case, the opposite. The concrete implementations are tightly coupled to its super class. My main problem with that, you can neither test the abstraction without a implementation, nor can you test a implementation without its abstraction.

Further, you're violating Liskov's substitution law (The L in SOLID): If your program works with type T, it should work with U extends T interchangeably, without changing your application. I think it's enough to explicitly declare a Dealer and a Player in that case. I hope someone corrects me, if I'm wrong, or describes it better: I always had a hard time understand the L and I usually have other reasons to avoid inheritance.

Another general rule is composition over inheritance. Meaning: Do not use inheritance for code reuse. It's much more flexible, easier to maintain and what not.

Hope that helps...

• Deck: I can't see any declaration of that type.
• Deck.shuffle(): Is only called within the implementation, so no need to make that public.
• AbstractPlayer: is not abstract
• AbstractPlayer: The hand is not private (it is implementation of abstraction, and should not be accessible to implementation of the abstraction... if that makes any sense)
• The Card itself should be displayed by the Prompter, too (think of adding a GUI)
• CardDeck: The Constructor has code duplication. The upper one should call the lower one with 1 as parameter. So initializeMultipleDecks will be initializeDecks, and initializeDeck can be removed. Then, you also get rid of the duplicate shuffle() call.
• CardDeck: Not sure if you have to pass the Suit[] and Rank[] array for the initializer methods. I think it's okay, to get the values from the enum itself. (If you want to enhance it later: Add the method with the two parameters again, and delegate the call from the no-param method to the newly added one).
• CardDeck.dealCard: There's a 'isEmpty' method in List. And: I can't find the place, where you actually check, if there's no card left? So, you will always end with an exception.
• CardDeck.dealCard: return deck.remove(0) does the job.
• Player.performAction: The method name doesn't tell me, what the method should do - but I see, this one is a bit tricky.
• Player.performAction: I think the loop can be simplified: Can't the if condition moved into the while condition? Also: You have an empty default case.
• Prompter.getState: State is not clear. The "H" and "S" is duplicated, that should be a constant with a proper name - maybe even provided in the enum itself (String symbol as constructor of the enum?). Also: the default case will return null. Since the code won't ever reach that point, I'd throw an Exception - if that happens, a bug has been introduced.
• Prompter.ask: Who asks what whom why?
• Rank: Can't remember, if I have ever seen a non-private constructor in an enum and I didn't know you can actually do that. However, it doesn't make any sense, since noone else can call it anyway?

inheritance

A main object oriented prinipcle is "lose coupling", inheritance can be, as I think it is in your case, the opposite. The concrete implementations are tightly coupled to its super class. My main problem with that, you can neither test the abstraction without a implementation, nor can you test a implementation without its abstraction.

• Deck: I can't see any declaration of that type.
• Deck.shuffle(): Is only called within the implementation, so no need to make that public.
• AbstractPlayer: is not abstract
• AbstractPlayer: The hand is not private (it is implementation of abstraction, and should not be accessible to implementation of the abstraction... if that makes any sense)
• The Card itself should be displayed by the Prompter, too (think of adding a GUI)
• CardDeck: The Constructor has code duplication. The upper one should call the lower one with 1 as parameter. So initializeMultipleDecks will be initializeDecks, and initializeDeck can be removed. Then, you also get rid of the duplicate shuffle() call.
• CardDeck: Not sure if you have to pass the Suit[] and Rank[] array for the initializer methods. I think it's okay, to get the values from the enum itself. (If you want to enhance it later: Add the method with the two parameters again, and delegate the call from the no-param method to the newly added one).
• CardDeck.dealCard: There's a 'isEmpty' method in List. And: I can't find the place, where you actually check, if there's no card left? So, you will always end with an exception.
• CardDeck.dealCard: return deck.remove(0) does the job.
• Player.performAction: The method name doesn't tell me, what the method should do - but I see, this one is a bit tricky.
• Player.performAction: I think the loop can be simplified: Can't the if condition moved into the while condition? Also: You have an empty default case.
• Prompter.getState: State is not clear. The "H" and "S" is duplicated, that should be a constant with a proper name - maybe even provided in the enum itself (String symbol as constructor of the enum?). Also: the default case will return null. Since the code won't ever reach that point, I'd throw an Exception - if that happens, a bug has been introduced.
• Prompter.ask: Who asks what whom why?
• Rank: Can't remember, if I have ever seen a non-private constructor in an enum and I didn't know you can actually do that. However, it doesn't make any sense, since noone else can call it anyway?

Inheritance

A main object oriented principle is "lose coupling", inheritance can be, as I think it is in your case, the opposite. The concrete implementations are tightly coupled to its super class. My main problem with that, you can neither test the abstraction without a implementation, nor can you test a implementation without its abstraction.