7
\$\begingroup\$

You can play this game as much as you want and after every game, you win/loss is recorded. Javascript has -0 and +0 so I guess that's why the counter acts weird at the beginning. Took me 4 months to do this (from never having heard of objects to this : feels good). I can't load images here at codereview so the link to the site is hosted here.

BlackJack

'use strict'
// dom elements and event handlers
var deal = document.getElementById('dealBtn')
var hit = document.getElementById('hitBtn')
var stand = document.getElementById('standBtn')
var reset = document.getElementById('resetBtn')
deal.addEventListener('click', playGame)
hit.addEventListener('click', goToHitMethod)
stand.addEventListener('click', userStands)
reset.addEventListener('click', resetGame)
var playerSum = document.getElementById('playersum')
var dealerSum = document.getElementById('dealersum')
var writeResult = document.getElementById('resultbox')
var winsCounter = document.getElementById('winscounter')
var playerCards = document.getElementById('playercards')
var dealerCards = document.getElementById('dealercards')
var noOfCardsPlayer = 0,
  noOfCardsDealer = 0,
  noOfWins = 0

// Defining properties and methods for every single card object created by fillPlayingCards function
function CardObject(cardNum, cardSuit) {
  this.cardNum = cardNum
  this.cardSuit = cardSuit
}
CardObject.prototype.getCardValue = function() {
  if (this.cardNum === 'jack' || this.cardNum === 'queen' || this.cardNum === 'king') {
    return 10
  } else if (this.cardNum === 'ace') {
    return 11
  } else {
    return this.cardNum
  }
}

// Deck object constructer with its properties and methods
function DeckObject() {
  this.iniDeck = []
  this.displayCards = function(cards) { // displaying corresponding card images on DOM
    // var fragment = document.createDocumentFragment()
    for (var i = 0; i < cards.length; i++) {
      var imgElement = document.createElement('img')
      imgElement.src = 'Images/' + cards[i].cardNum + '_of_' + cards[i].cardSuit + '.png'
      imgElement.style.height = '120px'
      imgElement.style.width = '100px'
        // fragment.appendChild(imgElement) only child nodes of fragment are added on DOM //
      if (this === mainPlayer) {
        noOfCardsPlayer++
        playerCards.appendChild(imgElement)
      } else {
        noOfCardsDealer++
        dealerCards.appendChild(imgElement)
      }
    }
  }
  this.sumCards = function(cards) { // adding numeric values of given cards
    var sum = 0,
      aces = 0
    for (var i = 0; i < cards.length; i++) {
      if (cards[i].getCardValue() === 11) { // checking for aces if >21, sum is decreased by 10
        aces += 1
        sum = sum + cards[i].getCardValue()
      } else {
        sum = sum + cards[i].getCardValue()
      }
    }
    while (aces > 0 && sum > 21) {
      aces -= 1
      sum -= 10
    }
    return sum
  }
  this.hitCard = function(cards) {
    var soloCard = [] // when we extract the last card, it comes off as an object. So we store that obj here inside this array to be able to pass it to displayCard() function
    var extraCard = cards.push(PlayingDeck.iniDeck.pop())
    soloCard.push(cards[extraCard - 1]) // push only the last added card and display it
    this.displayCards(soloCard)
    if (this === mainPlayer) {
      checkIfBust()
    }
  }
}

// Main deck used to play the game
var PlayingDeck = new DeckObject();
(function fillPlayingDeck() { // Filling the main deck with card objects
  var listCardNum = ['ace', 2, 3, 4, 5, 6, 7, 8, 9, 10, 'jack', 'queen', 'king']
  var listCardSuits = ['clubs', 'diamonds', 'hearts', 'spades']
  for (var i = 0; i < listCardNum.length; i++) {
    for (var j = 0; j < listCardSuits.length; j++) {
      PlayingDeck.iniDeck.push(new CardObject(listCardNum[i], listCardSuits[j])) // generating 52 new card objects
    }
  }
  var len = PlayingDeck.iniDeck.length,
    randomNum, tempValue
  while (len) { // Fischer-Yates shuffling Algorithm
    randomNum = Math.floor(Math.random() * len--)
    tempValue = PlayingDeck.iniDeck[len]
    PlayingDeck.iniDeck[len] = PlayingDeck.iniDeck[randomNum]
    PlayingDeck.iniDeck[randomNum] = tempValue
  }
}())

// player and dealer function objects
var mainPlayer = new DeckObject()

function player() {
  mainPlayer.iniDeck.push(PlayingDeck.iniDeck.pop(), PlayingDeck.iniDeck.pop())
  mainPlayer.displayCards(mainPlayer.iniDeck)
  playerSum.value = mainPlayer.sumCards(mainPlayer.iniDeck)
}

var mainDealer = new DeckObject()

function dealer() {
  mainDealer.iniDeck.push(PlayingDeck.iniDeck.pop(), PlayingDeck.iniDeck.pop())
  mainDealer.displayCards(mainDealer.iniDeck)
  dealerSum.value = mainDealer.sumCards(mainDealer.iniDeck)
}

// function that compares if player has busted or not everytime he/she hits
function checkIfBust() {
  var playerScore = mainPlayer.sumCards(mainPlayer.iniDeck)
  var dealerScore = mainDealer.sumCards(mainDealer.iniDeck)
  playerSum.value = playerScore
  dealerSum.value = dealerScore
  if (playerScore > 21) {
    writeResult.value = 'You BUSTED !!'
    noOfWins -= 1
    winsCounter.value = noOfWins
    disableHitStand()
  } else if (playerScore === 21) {
    writeResult.value = 'It\'s 21. You win !!'
    noOfWins += 1
    winsCounter.value = noOfWins
    disableHitStand()
  }
}

// function that runs on hit
function goToHitMethod() {
  mainPlayer.hitCard(mainPlayer.iniDeck)
}

// function that runs if user stands
function userStands() {
  var playerScore = mainPlayer.sumCards(mainPlayer.iniDeck)
  var dealerScore = mainDealer.sumCards(mainDealer.iniDeck)
  playerSum.value = playerScore
  while (dealerScore < 17) {
    mainDealer.hitCard(mainDealer.iniDeck)
    dealerScore = mainDealer.sumCards(mainDealer.iniDeck)
    dealerSum.value = dealerScore
  }
  if (dealerScore > playerScore && dealerScore <= 21) {
    writeResult.value = 'Dealer won with ' + dealerScore
    noOfWins -= 1
    winsCounter.value = noOfWins
    disableHitStand()
  } else if (playerScore > dealerScore || dealerScore > 21) {
    if (playerScore === 21) {
      writeResult.value = 'You won with BLACKJACK !'
      noOfWins += 1
      winsCounter.value = noOfWins
      disableHitStand()
    } else {
      writeResult.value = 'You won with ' + playerScore
      noOfWins += 1
      winsCounter.value = noOfWins
      disableHitStand()
    }
  } else {
    writeResult.value = 'Both tied with ' + playerScore
    disableHitStand()
  }
}

// function that disables hit and stand button after result is shown.
function disableHitStand() {
  stand.disabled = true
  hit.disabled = true
}

// main game function on 'deal' button click
function playGame() {
  player()
  dealer()
  deal.disabled = true
  stand.disabled = false
  hit.disabled = false
}

// game reset on 'reset' button click
function resetGame() {
  writeResult.value = ''
  dealerSum.value = ''
  playerSum.value = ''
  deal.disabled = false;
  (function removeImages() {
    var playerCardImages = playerCards.childNodes
    var dealerCardImages = dealerCards.childNodes
    for (var i = noOfCardsPlayer; i > 0; i--) {
      playerCardImages[i].parentNode.removeChild(playerCardImages[i])
    }
    for (var j = noOfCardsDealer; j > 0; j--) {
      dealerCardImages[j].parentNode.removeChild(dealerCardImages[j])
    }
  }())
  mainPlayer.iniDeck = []
  mainDealer.iniDeck = []
  noOfCardsDealer = 0
  noOfCardsPlayer = 0
}
html {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
  font-family: "Roboto", "sans-serif";
  text-rendering: optimizeLegibility;
  font-weight: 200;
  font-size: 20px;
  color: #ffffff;
}
body {
  background-color: #112b42;
}
h2 {
  text-align: center;
  margin: 0 auto;
}
#container {
  background-color: #562308;
  margin: 1em;
  padding: 1%;
  width: 50%;
  margin-left: 25%;
}
.images {
  background-color: #112b42;
  padding: 1%;
}
#playercards {
  margin: 0 auto;
  padding: 0.5%;
  background-color: #31172a;
  height: 125px;
}
#dealercards {
  margin: 0 auto;
  padding: 1%;
  background-color: #062c09;
  height: 125px;
}
.dealreset {
  display: block;
  text-align: center;
}
button.truncate {
  overflow: hidden;
  text-overflow: ellipsis;
  white-space: nowrap;
}
#dealBtn,
#resetBtn {
  width: 15%;
  height: 10%;
  display: inline-block;
  position: relative;
  margin: 1% 30%;
  border-radius: 20px 2px;
  overflow-wrap: break-word;
  /* when text-overflows break the letters and wrap in new-lines */
}
.hitstand {
  display: block;
  text-align: center;
}
#hitBtn,
#standBtn {
  width: 15%;
  height: 10%;
  display: inline-block;
  margin: 0.5% 15%;
  position: relative;
  border-radius: 1% 2%;
  overflow-x: hidden;
  /* hide text that overflows in x-axis */
  text-overflow: ellipsis;
  /* add ... in button to show that some text is there */
}
input[type=text] {
  border-radius: 5%;
}
.sumdisplay {
  display: block;
  text-align: center;
}
#playersum,
#dealersum {
  display: block;
  margin: 1% auto;
  font-weight: bold;
  text-align: center;
  font-size: 16px;
  padding: 1px;
  max-width: 100%
}
#show {
  margin-top: 1%;
  text-align: left;
  display: inline-block;
}
#resultbox {
  display: inline-block;
  max-width: 85%;
  font-size: 15px;
  font-weight: bold;
  color: #5500ff;
}
#counter {
  display: inline-block;
  margin-top: 1%;
  text-align: center;
}
#winscounter {
  display: inline-block;
  font-size: 15px;
  font-weight: bold;
  max-width: 10%;
  border: solid;
  border-radius: 20%;
  padding: 1px;
  text-align: center;
}
footer {
  text-align: center;
  font-family: Impact;
  position: relative;
}
a:link {
  text-decoration: none;
}
a:hover,
a:visited {
  color: red;
}
<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8">
  <title>Blackjack game by Bijay</title>
  <link rel="stylesheet" type="text/css" href="blackjack.css">
</head>

<body>
  <h2>♠ bLaCkJaCk ♥</h2>
  <div id="container">
    <div class="images">
      <div id="playercards">
      </div>
      <div id="dealercards">
      </div>
    </div>
    <div class="buttons">
      <div class="dealreset">
        <button id="dealBtn">Deal</button>
        <button id="resetBtn">Reset</button>
      </div>
      <div class="hitstand">
        <button id="hitBtn">Hit</button>
        <button id="standBtn">Stand</button>
      </div>
    </div>
    <div class="sumdisplay">
      Your score :
      <input type="text" id="playersum" value="">Dealer score :
      <input type="text" id="dealersum" value="">
    </div>
    <div class="result">
      <div id="show">Result :
        <input type="text" id="resultbox">
      </div>
      <div id="counter">Wins Counter :
        <input type="number" id="winscounter">
      </div>
    </div>
  </div>
  <footer>
    <p>You can find the source code at my<a href="https://github.com/bijay007/BlackJack"> GitHub </a>repository</p>
  </footer>
</body>
<script src="blackjack.js"></script>

</html>

\$\endgroup\$
  • \$\begingroup\$ I meant I couldn't upload images here at codereview site, but the game won't throw error. I have polished it even more with a newer version with better win-counter, controlled html elements overflow when window is super-minimized and so on. I have it static hosted at aws s3 and its public. \$\endgroup\$ – Bijay Timilsina Jan 22 '17 at 9:01
  • \$\begingroup\$ I've added a screenshot. \$\endgroup\$ – t3chb0t Jan 22 '17 at 9:17
  • \$\begingroup\$ When I play it on awsI only get 10-11 hands before it becomes unresponsive. Have you encountered this? \$\endgroup\$ – Raystafarian Jan 22 '17 at 14:54
  • \$\begingroup\$ @Raystafarian you are right :) it is because the all the card in the deck are finished after many runs and thus need to be reshuffled again. i will add this funcionality soon. thank you :) \$\endgroup\$ – Bijay Timilsina Jan 22 '17 at 21:29
  • \$\begingroup\$ Ah! Using a single deck, makes sense. \$\endgroup\$ – Raystafarian Jan 22 '17 at 21:36
4
\$\begingroup\$

Good job on writing a full game! But I do have a review:

Semi-colons

JavaScript lines should end with a semi-colon. Your code works, yes, but that's because the interpreter figures out where semicolons should go, and inserts them itself. I don't like relying on that. It can lead to some weird bugs.

To me, it's a bit like lazy texting, like "r u there yet". Yes it gets the point across but it's not English. If you're writing code, take it seriously and be formal.

Also, you current code can't necessarily be minified. Naïve minification will just remove whitespace and linebreaks, so without semicolons your code just becomes one long line of run-on nonsense. Good minifiers don't suffer from this, though.

(Edit: Before the flamewars start, yes, this is my opinion as much as anything. In the comments OP mentions the "standardjs" style guide, which says "No semicolons". I just plain disagree. But I encourage anyone to read standardjs's references and reasons as well, and form their own opinion. Meanwhile, I'll still use semicolons, and I'll keep encouraging others to do the same.)

Naming

Giving things an *Object suffix is redundant - and sort of incorrect. It's redundant because, well, object-oriented programming uses objects. What's interesting is what those object represent. So your DeckObject should just be called Deck. It doesn't represent a "deck object", it represents a deck.

The suffix is also sort of incorrect because DeckObject and CardObject are constructors: They instantiate objects rather than being objects themselves1. It'd make more sense to say deckObject = new Deck();.

Also, PlayingDeck shouldn't be capitalized, since it's not a constructor. It's just a variable, and so should be playingDeck.

Prototyping

Your DeckObject constructor essentially creates a blank object, and then adds all sorts of methods to it with this.sumCards = ... etc.. There's no real reason for this, as none of the functions use closures to simulate private variables or anything like that.

Your CardObject constructor does better: It adds its methods as prototype methods. DeckObject should do the same with its methods.

The difference is very subtle in practice (and in your particular code it doesn't actually make any difference what approach you take). But think of the prototype as the template for instances: Methods on the prototype automagically exist on every instance based on that prototype.
Conversely, methods added in constructor are only added to the instance being constructed. In this case, since it's done in the constructor, all instances will - in your case - still end up having the same methods, and they'll behave the same, but each instance will have its own copy of the methods - they're not part of the template.

Mixing responsibilities

Why is the DeckObject in charge of figuring out image URLs? The images are for the cards, so it'd make more sense for the cards to have that responsibility. The images belong to the cards; not the deck.

Similarly, why isn't the deck responsible for shuffling itself? Calling someDeck.shuffle() would be more natural.

Also, for some reason the DeckObject methods like sumCards take an array of cards. Why? If it's a deck, doesn't it have its own cards? It seems you're not using the deck to model a collection of cards, but as a namespace for methods instead. Which sort of defeats the point. You get things like:

dealerSum.value = mainDealer.sumCards(mainDealer.iniDeck)

when it'd make more sense to write:

dealerSum.value = mainDealer.sum();

(Incidentally, why is it called main dealer? Is there a non-main dealer in a game of Blackjack? Again, the naming is a little odd.)

You also have functions that do too much compared to their names. For instance, checkIfBust does check if the player busted, but it also tallies the score and updates the UI. Not what it says on the tin. Such mixing of concerns also makes it more difficult to change things later. E.g. if you just want to check if a hand is bust, you can't do that without either triggering a UI update, repeating code to check a hand's score, or refactoring checkIfBust to not do so many different things.

I'd separate the game logic from the UI concerns. Create something that that can just play Blackjack in the console, and then worry about UI and presentation. Build a support structure around your core Blackjack "engine" to display the game state.

Domain modelling

What "objects" does a Blackjack game consist of? Deck, dealer, player, and hands. The latter isn't modelled in your code? Instead you model your players as decks, which is a little... off.

True, both are collections of cards, but a hand has a score/sum, a deck does not; a hand can receive new cards, a deck cannot; cards can be drawn from a deck, but not from a hand; decks can be shuffled, hands cannot. And so on. So the two models are very different.

I'd do something like this (this is just a sketch/skeleton):

function Card(suit, value) {
  this.suit = suit;
  this.value = value;
  this.imageUrl = ...;
}

function Deck() {
  this.cards = ...; // create a deck of Card objects
  this.shuffle();
}

Deck.prototype = {
  // shuffles the deck
  shuffle: function () { ... },

  // pops a card of the stack
  draw: function () { ... }
};

function Hand() {
  this.cards = [];
}

Hand.prototype = {
  // adds a card to the hand, e.g. hand.addCard(deck.draw())
  addCard: function (card) { ... },

  // checks if bust
  isBust: function () { ... },

  // checks for blackjack
  isBlackjack: function () { ... },

  // sums card values
  score: function () { ... }
};

Rules

One nit-pick: A blackjack typically means Ace + face card. You can get 21 using many other combinations, but a proper blackjack beats a combined 21. So if I get 10+7+4, and dealer gets A+J, the dealer wins. Your code calls any sum of 21 "blackjack"... except if the dealer gets that score, in which case it just says 21. This also tells me (without looking at the code), that you're handling scoring differently for the player and the dealer, even though it should be the same logic.


1 Before someone nit-picks: Yes, constructors are functions, and functions in JavaScript are objects. So DeckObject can be said to be a correct name, because a constructor is an object... but again, redundancy. If anything it should then be DeckConstructorFunctionObject

\$\endgroup\$
  • \$\begingroup\$ Hello @Flambino .Thank you very much for such a detailed review. Seeing all the errors that I've made which you pointed out, made me feel real happy as i live by this code . My knowledge on oop is limited (few months), so I'll try to address some of my decision-makings at the time of me writing this code. 1. semi-colon: i tried to adopt standardjs coding patterns but what you said about being unminifiable is true. i need to revert back i guess. 2. objNaming : absolutely true.need to get rid of the habit ... \$\endgroup\$ – Bijay Timilsina Jan 22 '17 at 21:53
  • \$\begingroup\$ @BijayTimilsina Glad to hear you found it useful! You're definitely on the right track, just keep coding (and looking at other people's code). As for the semicolons thing, it's a style thing - I just prefer vastly using them. Not having semi-colons can bite you, but it won't necessarily, and minifiers generally work fine. Standardjs is a fine style guide, but on this point I just disagree. Read their references (which argue the case for no semicolons), and form your own opinion. It really is opinion as much as anything. \$\endgroup\$ – Flambino Jan 22 '17 at 22:07
  • \$\begingroup\$ 3. prototyping : didn't understand the first half (sry my lack of knowledge). In the second part you said about adding all methods as prototype of DeckObject, which now I think was really the part which felt most weird writing. thanks for pointing that out :) 4. absolutely wonderful point. i was trying to 'bandage' every part which tried to show any error, so the code management process went rampant after creating the first few objects. .. \$\endgroup\$ – Bijay Timilsina Jan 22 '17 at 22:10
  • \$\begingroup\$ ....My thought at the time was to create a Deck with all methods in it for manipulating cards. Then create an array of 52 Card and push it in 'playingDeck' variable which is an instance of Deck. Then I created player and dealer objects where I pushed cards and called the methods from its constructer. You have put a ton of wonderful points there. i will need some days to slowly take all the good stuffs in :) 5.domain modeling - guess I needed to draw some mental framework like you did before spamming codes. thanks a lot for such a great review :) \$\endgroup\$ – Bijay Timilsina Jan 22 '17 at 22:10
  • \$\begingroup\$ @BijayTimilsina Yeah, prototypes are a tricky at first. Your DeckObject constructor essentially adds a bunch of methods to an otherwise empty instance when you instantiate it. CardObject on the other hand has its methods as prototype methods, which means they're built in to every card instance, rather than being added to each instance. The difference is very subtle. With this code, there's no real difference between the two, but you can do some tricks using instance-added methods (the closures I mention), that you can't do with prototypal methods. But worry about that later. \$\endgroup\$ – Flambino Jan 22 '17 at 22:32

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.