1
\$\begingroup\$

Original Question

I have updated my code based on feedback from other users and am submitting this question for re-evaluation.

HTML

<!doctype html>
<html lang="en">
    <head>
        <title></title>
        <link type="text/css" rel="stylesheet" href=styles.css />
    </head>
    <body>
        <div id="player1CurrentCard" class="card">
            <div class="warDeck">
                <div class="warCardsHolder"></div>
                <div class="text">
                    <p>War Deck</p>
                </div>
            </div>
            <div class="cardHolder"></div>
            <div class="text">
                <p>Player 1 Card</p>
            </div>
            <div id="player1CurrentDeck" class="currentDeck">
                <div class="currentCardsHolder"></div>
                <div class="text">
                    <p>Current Deck</p>
                </div>
            </div>
            <div id="player1WonDeck" class="wonDeck">
                <div class="wonCardsHolder"></div>
                <div class="text">
                    <p>Won Deck</p>
                </div>
            </div>

        </div>

        <div id="player2CurrentCard" class="card">
            <div class="cardHolder"></div>
            <div class="warDeck">
                <div class="warCardsHolder"></div>
                <div class="text">
                    <p>War Deck</p>
                </div>
            </div>
            <div class="text">
                <p>Player 2 Card</p>
            </div>
            <div id="player2CurrentDeck" class="currentDeck">
                <div class="currentCardsHolder"></div>
                <div class="text">
                    <p>Current Deck</p>
                </div>
            </div>
            <div id="player2WonDeck" class="wonDeck">
                <div class="wonCardsHolder"></div>
                <div class="text">
                    <p>Won Deck</p>
                </div>
            </div>
        </div>
        <button id="play">Play</button>
        <button id="reshuffle">Reshuffle</button>
    </body>
    <script src="classes.js"></script>
    <script src="war.js"></script>
</html>

CSS

.card{
    position: relative;
    float: left;
    width: 350px;
    height: 500px;
    text-align: center;
}

.wonDeck{
    position: absolute;
    left: 100px;
    bottom: 0;
    width: 75px;
    height: 100px;
    text-align: center;
}

.currentDeck{
    position: absolute;
    bottom: 0;
    width: 75px;
    height: 100px;
    text-align: center;
}

.card{
    margin: 0 5%;
}

.card:first-of-type{
    margin-left: 0;
}

.card:last-of-type{
    margin-right: 0;
}

.card .text{
    position: absolute;
    margin: 0 0 0 -25%;
    left: 35%;
    height: 30%;
    width: 50%;
    font-size: 26px;
    color: rgb(150, 150, 150);
}

.wonDeck .text, .currentDeck .text{
    position: absolute;
    margin: 0 0 0 -25%;
    left: 50%;
    height: 30%;
    width: 50%;
    font-size: 16px;
    color: rgb(150, 150, 150);
}

.warDeck .text{
    position: absolute;
    margin: 15% 0 0 -25%;
    left: 50%;
    height: 30%;
    width: 50%;
    font-size: 16px;
    color: rgb(150, 150, 150);
}

.cardHolder{
    position: absolute;
    top: 0;
    left: 0;
    width: 75%;
    height: 350px;
    font-size: 26px;
    color: rgb(0, 0, 0);
    border: 1px dashed black;
    background-color: rgba(0, 0, 0, 0.5);
}

.wonCardsHolder, .warCardsHolder, .currentCardsHolder{
    position: absolute;
    top: 0;
    left: 0;
    width: 100%;
    height: 100%;
    font-size: 16px;
    color: rgb(0, 0, 0);
    border: 1px dashed black;
    background-color: rgba(0, 0, 0, 0.5);
}


.warDeck{
    position: relative;
    float: right;
    width: 75px;
    height: 100px;
}

war.js

var play = document.getElementById("play");
var reshuffle = document.getElementById("reshuffle");
var cardHolder = document.getElementsByClassName("cardHolder");
var currentCardsHolder = document.getElementsByClassName("currentCardsHolder");
var wonCardsHolder = document.getElementsByClassName("wonCardsHolder");
var warCardsHolder = document.getElementsByClassName("warCardsHolder");

window.onload = function(){
    Player1 = new Player("Player 1", [], []);
    Player2 = new Player("Player 2", [], []);
    Deck.StartGame(Player1, Player2);
}
play.onclick = function(){
    PlayGame(Player1, Player2);
}

classes.js

function Player(name, currentDeck, wonDeck){
    this.name = name;
    this.currentDeck = currentDeck;
    this.wonDeck = wonDeck;
}
function Card(options){
    this.suit = options.suit;
    this.faceValue = options.faceValue;
    this.cardText = (function(){
        switch(this.faceValue){
            case 14:
                {return "Ace"};
            break;
            case 13:
                {return "King"};
            break;
            case 12:
                {return "Queen"};
            break;
            case 11:
                {return "Jack"};
            break;
            default:
                {return String(this.faceValue);}
            break;
        }
    }).call(this);
}
Player.prototype.GetCurrentCard = function(){
    this.currentCard = this.currentDeck.shift();
}
Deck = {
    suits: ["Clubs", "Diamonds", "Hearts", "Spades"],
    cards: [14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2],
    deck: [],
    shuffledDeck: [],
    BuildDeck: function(){
        for(var suit = 0; suit < this.suits.length; suit++){
            for(var card = 0; card < this.cards.length; card++){
                this.deck.push(new Card({suit: this.suits[suit], faceValue: this.cards[card]}));
            }
        }
    },
    ShuffleDeck: function(unshuffledDeck, shuffledDeck){
        while(unshuffledDeck.length){
            var index = Math.floor(Math.random() * unshuffledDeck.length); 
            shuffledDeck.push(unshuffledDeck.splice(index, 1)[0]);    
        }
        unshuffledDeck = [];
    },
    DistributeCards: function(player1Deck, player2Deck){
        for(var i = 0; i < this.shuffledDeck.length / 2; i++){
            player1Deck.push(this.shuffledDeck[i]);
            player2Deck.push(this.shuffledDeck[this.shuffledDeck.length - i - 1]);
        }
    },
    DealWarCards: function(player, warDeck, num){
        for(var i = 0; i < num; i++){
            player.GetCurrentCard();
            warDeck.push(player.currentCard);
        }
        return warDeck;
    },
    StartGame: function(player1, player2){
        this.BuildDeck();
        this.ShuffleDeck(this.deck, this.shuffledDeck);
        this.DistributeCards(player1.currentDeck, player2.currentDeck);
    }
}
function PlayGame(player1, player2){
    var player1WarDeck = [];
    var player2WarDeck = [];
    function GoToWar(){
        console.log("War");
        Deck.DealWarCards(player1, player1WarDeck, 2);
        Deck.DealWarCards(player2, player2WarDeck, 2);
        console.log(player1WarDeck, player2WarDeck);

        if(player1WarDeck[player1WarDeck.length - 1].faceValue === player2WarDeck[player2WarDeck.length - 1].faceValue){
            console.log("Tie");
            Deck.DealWarCards(player1, player1WarDeck, 2);
            Deck.DealWarCards(player2, player2WarDeck, 2);
            GoToWar();
        }
        if(player1WarDeck[player1WarDeck.length - 1].faceValue > player2WarDeck[player2WarDeck.length - 1].faceValue){
            player1.wonDeck = player1.wonDeck.concat(player1WarDeck, player2WarDeck);
            console.log("Player 1 wins");
        }
        else{
            player2.wonDeck = player2.wonDeck.concat(player1WarDeck, player2WarDeck);
            console.log("Player 2 wins");
        }
        warCardsHolder[0].textContent = player1WarDeck[player1WarDeck.length - 1].cardText +" of " +player1WarDeck[player1WarDeck.length - 1].suit;
        warCardsHolder[1].textContent = player2WarDeck[player2WarDeck.length - 1].cardText +" of " +player2WarDeck[player2WarDeck.length - 1].suit;
        cardHolder[0].textContent = player1WarDeck[0].cardText +" of " +player1WarDeck[0].suit;
        cardHolder[1].textContent = player2WarDeck[0].cardText +" of " +player2WarDeck[0].suit;
    }
    if(player1.currentDeck.length === 0){
        ReshuffleDeck(player1);
    }
    else{
        player1.GetCurrentCard();
    }
    if(player2.currentDeck.length === 0){
        ReshuffleDeck(player2);
    }
    else{
        player2.GetCurrentCard();
    }
    if(player1.currentCard.faceValue > player2.currentCard.faceValue){
        player1.wonDeck.push(player1.currentCard);
        player1.wonDeck.push(player2.currentCard);
    }
    else{
        player2.wonDeck.push(player2.currentCard);
        player2.wonDeck.push(player1.currentCard);
    }
    if(player1.currentCard.faceValue === player2.currentCard.faceValue){
        player1WarDeck.push(player1.currentCard);
        player2WarDeck.push(player2.currentCard);
        GoToWar();
    }
    else{
        cardHolder[0].textContent = player1.currentCard.cardText +" of " +player1.currentCard.suit;
        cardHolder[1].textContent = player2.currentCard.cardText +" of " +player2.currentCard.suit;
        currentCardsHolder[0].textContent = player1.currentDeck.length;
        currentCardsHolder[1].textContent = player2.currentDeck.length;
        wonCardsHolder[0].textContent = player1.wonDeck.length;
        wonCardsHolder[1].textContent = player2.wonDeck.length;
        warCardsHolder[0].textContent = "";
        warCardsHolder[1].textContent = "";
    }
    if(player1.currentDeck.length === 52){
        GameOver(player1);
    }
    if(player2.currentDeck.length === 52){
        GameOver(player2);
    }
}
function ReshuffleDeck(player){
    Deck.ShuffleDeck(player.wonDeck, player.currentDeck);
}
function GameOver(player){
    console.log(player.name +" wins!");
}

Final Submission

\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review. In my opinion, you've posted this question too soon, since your other question was up for less than 24 hours. You should wait, in my opinion, until you have an accepted answer on your original question. New information always comes, at it's own pace. Making a new question so soon with few answers isn't always a good idea. Just leaving my tought on this one. \$\endgroup\$
    – Ismael Miguel
    Commented Feb 21, 2017 at 12:27
  • \$\begingroup\$ @IsmaelMiguel thank you for the advice. Normally I wouldn't post the updated version so soon, but I have a fairly short amount of time to get the project done(It has to be done by Thursday) and I'm trying to get as much feedback as possible before then. \$\endgroup\$
    – Robert
    Commented Feb 21, 2017 at 12:49

1 Answer 1

Reset to default
2
\$\begingroup\$

1) Default your player initialization values so you don't have to pass in empty arrays.

function Player(name, currentDeck, wonDeck){
    this.name = name;
    this.currentDeck = currentDeck != undefined && currentDeck instanceof Array ? currentDeck : [];
    this.wonDeck = wonDeck != undefined && wonDeck instanceof Array ? wonDeck: [];
}
var player1 = new Player("Player 1");

2) PlayGame, ReshuffleDeck, and GameOver are not classes, and thus should not be in classes.js

3) I intentionally omitted the break statements from the switch cases I recommended in your first post - the reason for this is that return will return from the function immediately, and thus will not fall through to the next case. Putting a break after a return statement is unreachable code and is unnecessary.

4) Not that it really matters in this particular instance, but getting the currentCard using .shift() grabs you the first item in the array, which would essentially be grabbing from the bottom of your stack of cards. .pop() would grab the card "on top".

\$\endgroup\$
4
  • \$\begingroup\$ I thought .pop() pulled the last element of an array. the player's current card should be the first element in the array, which is why I used .shift(). I did some reorganizing earlier and created a functions.js script that holds the PlayGame() and GameOver() functions as well as a couple more I made. I guess I should do another question later today or tomorrow to show everything I did. I never thought about initializing the arrays, thank you for that. Once again, thank you for your help and feedback. \$\endgroup\$
    – Robert
    Commented Feb 21, 2017 at 20:05
  • 1
    \$\begingroup\$ Well, if you think about dealing a stack of cards, they go in LIFO/FILO (last in, first out / first in, last out) order. Which is the definition of the data structure known as a stack in programming. So, if you are adding cards to an array that represents the player's deck of cards from index 0 --> n, they should come out in n --> 0 order to replicate the behavior of a physical stack of cards. Like I said, it doesn't really matter for this particular application, but it may be something that an employer looks at to see if you can make that connection and use the proper data structure. \$\endgroup\$
    – mhodges
    Commented Feb 21, 2017 at 20:11
  • \$\begingroup\$ I never thought of it that way. \$\endgroup\$
    – Robert
    Commented Feb 21, 2017 at 20:14
  • 1
    \$\begingroup\$ @Robert Also, I wouldn't advise having a functions.js, if you separate into too many files, it becomes a dependency and scoping nightmare as the project grows, unless you set up something like requireJS or another script-loader. I think those functions would be just fine in the war.jsfile \$\endgroup\$
    – mhodges
    Commented Feb 21, 2017 at 20:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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