As part of a recent interview, I was assigned to write a small BlackJack program. After submitting the solution, I received an answer that "the solution was functionally OK, but was way below their standards of quality code, especially coming from a person with significant experience" (I have 15 years experience).
I have posted my code in order to receive an honest critical review on the quality of the code. It is true that I wrote it while on vacation, but does it really suck so much? If so, how could I have written it differently?
public class Deck {
public enum CARD {
S2(2),S3(3),S4(4),S5(5),S6(6),S7(7),S8(8),S9(9),S10(10),SJ(10),SQ(10),SK(10),SA(11),
D2(2),D3(3),D4(4),D5(5),D6(6),D7(7),D8(8),D9(9),D10(10),DJ(10),DQ(10),DK(10),DA(11),
C2(2),C3(3),C4(4),C5(5),C6(6),C7(7),C8(8),C9(9),C10(10),CJ(10),CQ(10),CK(10),CA(11),
H2(2),H3(3),H4(4),H5(5),H6(6),H7(7),H8(8),H9(9),H10(10),HJ(10),HQ(10),HK(10),HA(11);
private int points;
private CARD(int points) {
this.points = points;
}
public int getPoints() {
return points;
}
}
private Stack<CARD> deck = new Stack<Deck.CARD>();
private Deck(Stack<CARD> deck) {
this.deck = deck;
}
private Deck(){
shuffleDeck();
}
private void shuffleDeck() {
// we need it mutable and the list provided by Arrays is Immutable
List<CARD> allCards = new ArrayList(Arrays.asList(CARD.values()));
int max = 52;
Random rand = new Random();
while (allCards.size()>0) {
CARD removedCard = allCards.remove(rand.nextInt(max)) ;
this.deck.add(removedCard);
--max;
}
}
public CARD draw() {
return deck.pop();
}
public static class Builder {
Stack<CARD> deck=null;
public static Builder createNew(){
return new Deck.Builder();
}
public Builder fromFile(File file) throws IOException {
Scanner scanner=null;
try {
scanner = new Scanner(file);
deck = new Stack<>();
for (String card : scanner.nextLine().split(", ")) {
deck.add(Enum.valueOf(CARD.class, card));
}
} finally {
if (scanner!=null) {
scanner.close();
}
}
return this;
}
public Builder fromStack(Stack stack) {
this.deck = stack;
return this;
}
public Builder validate() {
if (deck==null) {
return this;
}
if (deck.size()>52) {
throw new RuntimeException("Critical error, deck is not initialized correctly.You need 52 unique cards.");
}
HashSet<CARD> allUniqueCards = new HashSet<>(deck==null?new HashSet<>():deck);
// verify uniqueness of cards in case of 52 card condition is met but some are duplicates
if (deck==null || allUniqueCards.size()==52) {
return this;
}
// this is error that is not recoverable therefore we define it as RuntimeException
throw new RuntimeException("Critical error, deck is not initialized correctly.You need 52 unique cards.");
}
public Deck build() {
return deck==null? new Deck():new Deck(deck);
}
}
public String toString() {
String toString = deck.stream().map(t->{return t.toString()+", ";}).reduce("", (t,q)->{return t+q;});
if (!toString.isEmpty()) {
toString = toString.substring(0,toString.length()-2);
}
return toString;
}
}
Hand class
public class Hand {
// in general it is one idea more correct to be Set,
//but I am on holiday right now and I need to write the tests fast.
private List<CARD> hand = new ArrayList<>();
public boolean hasBlackJack() {
return score()==21;
}
public boolean has22(){
return score()==22;
}
public boolean lessThan17(){
return score()<17;
}
public boolean greaterThan(Hand hand) {
return this.score()>hand.score();
}
public boolean greaterThan21(){
return score()>21;
}
public void addCard(CARD card) {
hand.add(card);
}
private int score(){
return hand.stream().mapToInt(t->{return t.getPoints();}).sum();
}
public boolean empty() {
return hand.size()==0;
}
public String toString() {
// the sequential stream is not nessesary realy, but it makes it one idea easier to test later.
String toString = hand.stream().sequential().map(t->{return t.name();}).reduce("", (t,q)->{return t.isEmpty()?q: t+", " + q;});
return toString;
}
}
Player class
public class Player {
private String playerName =null;
private Hand hand = new Hand();
public Player(String playerName) {
this.playerName=playerName;
}
public boolean hasBlackJack(){
return hand.hasBlackJack();
}
public boolean has22() {
return hand.has22();
}
public boolean lessThan17(){
return hand.lessThan17();
}
public boolean greaterThan21() {
return hand.greaterThan21();
}
public boolean hasEmptyHand() {
return hand.empty();
}
public boolean handGreaterThan(Player player) {
return this.hand.greaterThan(player.hand);
}
public void draw(Deck deck) {
hand.addCard(deck.draw());
}
public String getPlayerName(){
return playerName;
}
public String toString() {
return playerName + ":" + " " + hand.toString();
}
}
//Assumption in case of a tie SAM wins.
public class Play21 {
Player dealer = null;
Player sam = null;
Deck deck = null;
public Play21(Player sam,Player dealer,Deck deck) {
this.sam=sam;
this.dealer=dealer;
this.deck = deck;
}
public static void printWinner(Player winner,Player looser) {
System.out.println(winner.getPlayerName());
System.out.println(winner.getPlayerName().equals("sam")?winner:looser);
System.out.println(winner.getPlayerName().equals("sam")?looser:winner);
}
private Player round1() {
sam.draw(deck);
dealer.draw(deck);
sam.draw(deck);
dealer.draw(deck);
if (dealer.hasBlackJack()&&sam.hasBlackJack()) {
return sam;
}
else if (sam.hasBlackJack()) {
return sam;
}
else if(dealer.hasBlackJack()) {
return dealer;
}
if (dealer.has22()&&sam.has22()) {
return dealer;
}
else if (sam.has22()) {
return dealer;
}
else if (dealer.has22()) {
return sam;
}
return null;
}
private Player round2() {
if (sam.hasEmptyHand()||dealer.hasEmptyHand()) {
throw new IllegalStateException("You need to invoke round1 first");
}
while(sam.lessThan17()) {
sam.draw(deck);
}
if (sam.greaterThan21()) {
return dealer;
}
while (sam.handGreaterThan(dealer)) {
dealer.draw(deck);
}
if (dealer.greaterThan21()) {
return sam;
}
if (dealer.handGreaterThan(sam)) {
return dealer;
}
else {
return sam;
}
}
public Player play(Deck deck){
Player winner = round1();
if (winner!=null) {
return winner;
}
return round2();
}
public static Deck createDeck(String args[]) throws IOException {
Deck deck =null;
if (args.length==0) {
deck=Deck.Builder.createNew()
.validate()
.build();
}
else if (args.length==1) {
File file = new File(args[0]);
if (file.isFile()) {
deck=Deck.Builder.createNew()
.fromFile(file)
.validate()
.build();
}
else {
// Unrecoverable exception therefore declared as RuntimeException
throw new RuntimeException("Path is not a file, or file does not exist,or file is unreadable(check permissions).");
}
}
else {
throw new RuntimeException("Illegal number of arguments. ");
}
return deck;
}
public static void main(String[] args) throws IOException{
Deck deck = createDeck(args);
Player dealer = new Player("dealer");
Player sam = new Player("sam");
Play21 play = new Play21(sam,dealer,deck);
Player winner = play.play(deck);
printWinner(winner, winner==sam?dealer:sam);
}
}
