Codecademy BlackJack project

I've put in quite a bit of hours on this BlackJack project for Codecademy to get it to the point where I'm satisfied with it. Before I move on to create the UI for it I would really appreciate feedback on things to improve, such as my use of classes and objects, best practices, etc. I'm including a link to my jsFiddle.

BlackJack project

```
"use strict";

// Hands for players, multiple hands used for splitting cards
function Hand() {
var cards = [],
// Set to true if totalScore > 21
isBust = false,
// Total of all cards at a point in time, aces are handled in calcTotalScore
totalScore = 0;

// Sets totalScore and isBust
this.calcTotalScore = function () {
var hasAce = false,
tempCardValue;

if (cards.length === 0) {
console.log("ERROR: No cards in this Hand");
} else {
// Reset totalScore before every calculation
totalScore = 0;

for (var i = 0; i 21) && hasAce) {
totalScore -= 10;
}

if (totalScore > 21) {
isBust = true;
}
}
}

this.popLastCard = function () {
var tempPoppedCard = cards.pop();
this.calcTotalScore();

return tempPoppedCard;
};

this.pushNewCard = function (card) {
cards.push(card);
this.calcTotalScore();
};

this.getScore = function () {
return totalScore;
};

this.getCard = function (cardPos) {
return cards[cardPos].getCardNumber() + " of " + cards[cardPos].getCardSuit();
};

this.getCardNumber = function (cardPos) {
return cards[cardPos].getCardNumber();
}

this.getCardsLength = function () {
return cards.length;
}

this.getIsBust = function () {
return isBust;
}
}

// Cards exist in Hands or Decks
function Card(inSuit, inNum) {
this.getCardSuit = function () {
var suitName = "";
switch (inSuit) {
c

Can you give me an example by refactoring one of the if statements?

Your Card object can be changed as follows:

Following your general style,

function Suit(name, sym) {
    this.getName = function () {
        return name;
    }

    this.getSymbol = function () {
        return sym;
    }
}

var hearts = new Suit("Hearts", "\u2665");
var clubs = new Suit("Clubs", "\u2663");
var diamonds = new Suit("Diamonds", "\u2666");
var spades = new Suit("Spades", "\u2660"); 
var suits = [hearts, clubs,  diamonds, spades];

function Rank(name, value) {
    this.getName = function() {
        return name;
    }

    this.getValue = function() {
        return value
    }
}

var ranks = [
    new Rank("Ace", 11),             
    new Rank("2", 2),             
    new Rank("3", 3),             
    new Rank("4", 4),             
    new Rank("5", 5),             
    new Rank("6", 6),
    new Rank("7", 7),
    new Rank("8", 8),
    new Rank("9", 9),
    new Rank("10", 10),
    new Rank("Jack", 10),
    new Rank("King", 10),
    new Rank("Queen", 10)
];

// Cards exist in Hands or Decks
function Card(suit, rank) {
    this.getCardSuit = function () {
        return suit.getName();
    };

    this.getCardNumber = function () {
        return rank.getName();
    };

    this.getCardValue = function () {
        return rank.getValue();
    };

}

And your deck creation accordingly changed from:

// Populate deck with 52 cards in order
for (var i = 0; i < 52; i++) {
    deckOfCards[i] = new Card(tempCardSuit, tempCardNumber);

    if (tempCardNumber < 13) {
        tempCardNumber++;
    } else {
        tempCardNumber = 1;
        tempCardSuit++;
    }
}

to:

for (var suitIdx = 0; suitIdx < suits.length; suitIdx ++) {
    for (var rankIdx = 0; rankIdx < ranks.length; rankIdx ++) {
        deckOfCards.push(new Card(suits[suitIdx], ranks[rankIdx]));
    }
}

Suit and Rank are Value Objects from your domain. In which you can put more behavior. Now you can change relatively more easily from suit names to suit symbols for example.

As a rule of thumb if a property does not change during the life time of an object, do not calculate it in the object, you may pass it to the constructor.

Another OOP tip: If you are calling a number of methods of an object in a row, those code should probably go into that object.

this.getCard = function (cardPos) {
    return cards[cardPos].getCardNumber() + " of " + cards[cardPos].getCardSuit();
};

the portion:

card.getCardNumber() + " of " + card.getCardSuit();

can go into card.toString()

Another one: Methods should be short and do one specific thing which should be indicated by their names.
Your Deck.shuffle() methods is doing multiple things at once. It both populates the deck, but it also shuffles it afterwards. Populating the deck should move out of shuffle method.

The same is true for you main() method. 2 players and a deck are asking to be moved into a class BlackJackGame or some other name.

DRY: Don't repeat yourself.
Repeating instances of dealer.pushNewCard(0, deck1.popLastCard()) should move in to a function. Also note that that function can be a method of an object that contains both deck and player. Also note that method names containing implementation details like push or pop and the magic constant 0 are clues that that behavior needs to be encapsulated.

Comments such as this:

// Display results

followed by a chunk of code should better be factored out into a method of its own, usually getting their names from the comment itself.

StackExchange Code Review Q#20259, answer score: 5