5
\$\begingroup\$

I am almost completing my BlackJack game in Javascript. But I realize that there must be a way to improve the performance for these two methods. Any pointers?

const hasAceInHand = (cardsOnHand) => {
    for (let key in cardsOnHand) {
        let arr = cardsOnHand[key];
        for (let i = 0; i < arr.length; i++) {
            let obj = arr[i];
            for (let prop in obj) {
                if (prop === "face") {
                    if (obj[prop] === "A") {
                        return true;
                    }
                }
            }
        }
    }
    return false;
}

const countHandValue = (cardsOnHand) => {
    //console.log(hasAceInHand(cardsOnHand));
    let sum = 0;
    for (let key in cardsOnHand) {
        let arr = cardsOnHand[key];
        for (let i = 0; i < arr.length; i++) {
            let obj = arr[i];
            for (let prop in obj) {
                if (prop === "faceValue") {
                    //console.log(prop + " = " + obj[prop]);
                    sum = sum + obj[prop];
                    debugger;
                    if (sum > 21 && hasAceInHand(cardsOnHand)) {
                        // Transfer Ace's face value from 11 to 1
                        sum = sum - 11;
                        sum = sum + 1;
                    }
                }
            }
        }
    }

    return sum;
}

This countHandValue method is called at below:

const cardSuit = ["hearts", "diams", "clubs", "spades"];
const cardFace = ["2", "3", "4", "5", "6", "7", "8", "9", "10", "J", "Q", "K", "A"];

// In JavaScript, functions are objects.
// You can work with functions as if they were objects.
function card(suit, face) {
    this.suit = suit;
    this.face = face;

    switch (face) {
        case "A":
            this.faceValue = 11;
            break;
        case "J":
        case "Q":
        case "K":
            this.faceValue = 10;
            break;
        default:
            this.faceValue = parseInt(face);
            break;
    }
};

const deck = {
    cards: []
}

var tempCard;
const player = {
    cards: [],
    handValue: 0
}

const dealer = {
    cards: [],
    handValue: 0
}

const dealOneCardToPlayer = () => {
    // Take a card from the top deck to be assigned to tempcard.
    tempCard = deck.cards.splice(0, 1);
    player.cards.push(tempCard);
    player.handValue = countHandValue(player.cards);
    document.getElementById("handValuePlayer").innerHTML = player.handValue;
    makeCardPlayer(tempCard[0]);
}

I have uploaded the whole program.

\$\endgroup\$
4
\$\begingroup\$

Review

Function hasAceInHand could be boiled down to a one-liner using array.find. It returns the first element of the array that passes the condition x => x.face === "A" provided.

const hasAceInHand = (cardsOnHand) => {
  return cardsOnHand.find(x => x.face === "A") != null;
}

Function countHandValue iterates the items, and calls the other function which iterates the items nested in the outer iteration if (sum > 21 && hasAceInHand(cardsOnHand)) { ... You could just keep track of aces to avoid iterating the items more than once.

Alternative for countHandValue

In black-jack, at most 1 ace could have value 11, all other aces in the hand must have value 1, else 21 would get exceeded (11 + 11 > 21). I would change the default value of an ace to 1.

switch (face) {
    case "A":
        this.faceValue = 1;
        break;

As a first step, Array.reduce could be used to calculate the sum of card values.

const countHandValue = (cardsOnHand) => {
  return cardsOnHand.reduce((sum, current) => {
    sum += current.faceValue;
    return sum;
  }, 0);
}

Then we can implement a relaxation allowing at most 1 ace to get value 11.

const countHandValue = (cardsOnHand) => {
  let ace;
  let value = cardsOnHand.reduce((sum, current) => {
    ace |= current.faceValue === 1;
    sum += current.faceValue;
    return sum;
  }, 0);
  if (ace && value + 10 <= 21) {
    value += 10;
  }
  return value;
}

And we also no longer need to call hasAceInHand as we're tracking the ace inline.

\$\endgroup\$
3
\$\begingroup\$

I'd suggest using for...of loops to iterate over the cards, but before we can do that, there is an important point with the dealOneCardToPlayer() function:

tempCard = deck.cards.splice(0, 1);

Note that Array.splice() returns "An array containing the deleted elements."1 and thus the name tempCard is misleading - perhaps a more appropriate name would be tempCards.

This means that the next line:

player.cards.push(tempCard); 

leads to player.cards having a nested array with that card. If instead the first element of that array returned from the call to .splice() was pushed into the players cards array, it would make those functions easier to work with, along with for...of loops. That way there is no need to create a counter variable, increment it after each iteration, and use it just to get the next element from the array.

Also, shorthand assignment operations like += and -= can be used instead of sum = sum - 11 and sum = sum + 1.

Also there is little need to loop through the properties:

for (let prop in obj) {
      if (prop === "face") {
          if (obj[prop] === "A") {

Just check the property directly:

if (obj.face === "A") {

If you want to ensure the property exists, use the in operator:

if ('face' in obj && obj.face === "A") {

Putting it all together:

const hasAceInHand = (cardsOnHand) => {
    for (const card of cardsOnHand) {
      if (card.face === "A") {
        return true;
      }
    }
    return false;
}

const countHandValue = (cardsOnHand) => {
    let sum = 0;
    for (const card of cardsOnHand) {
      sum = sum + card.faceValue;
      if (sum > 21 && hasAceInHand(cardsOnHand)) {
        // Transfer Ace's face value from 11 to 1
        sum -= 10; // - 11 + 1
      }
    }
    return sum;
}

1https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice#Return_value

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for the code review. After some thoughts, I think it is more efficient algorithm to transform Ace value during drawOneCard method. If total hand value plus current card draw card value is more than 21 and current draw card face is Ace, then transform current card value to 1. This applies to even the first two dealt cards because the first two cards could be Ace and Ace (double Aces). \$\endgroup\$ – Steve Ngai Sep 14 at 7:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.