We’re rewarding the question askers & reputations are being recalculated! Read more.
2 added 4 characters in body
source | link
const previousState = {
  phase: 'start', // start, in-game, end, etc.
  winner: null,
  deck: [ ...values... ],
  players: [
    { name: 'player1', hand: [/* empty hand */] },
    { name: 'player2', hand: [/* empty hand */] },
  ]
}

function shuffleDeck(state){
  return {
    // Copy everything over
    ...state,
    // But override deck
    deck: state.deck.reduce((shuffled, card) => {
      // For each card in the previous state's deck, insert them in random
      // positions in the array and return that array.
      shuffled.splice(Math.floor(Math.random() * shuffled.length), 0, card);
      return shuffled;
    }, []);
  }
}

const newState = shuffleDeck(state);

As you can see, shuffling the deck is a new state constructed from the old state, with the deck modified. From a testability point of view, you can easily craft the old state from a bunch of objects and arrays, feed it to the function and you get your result. You do not need to create instances of Deck, of Player, no mocking, no stepping through, none of that voodoo. In fact, this is already a test case. What's lacking is comparing previousState's values with newState's, seeing if deck was properly shuffled while nothing else changedthe assertions.

const previousState = {
  phase: 'start', // start, in-game, end, etc.
  winner: null,
  deck: [ ...values... ],
  players: [
    { name: 'player1', hand: [/* empty hand */] },
    { name: 'player2', hand: [/* empty hand */] },
  ]
}

function shuffleDeck(state){
  return {
    // Copy everything over
    ...state,
    // But override deck
    deck: state.deck.reduce((shuffled, card) => {
      // For each card in the previous state's deck, insert them in random
      // positions in the array and return that array.
      shuffled.splice(Math.floor(Math.random() * shuffled.length),card);
      return shuffled;
    }, []);
  }
}

const newState = shuffleDeck(state);

As you can see, shuffling the deck is a new state constructed from the old state, with the deck modified. From a testability point of view, you can easily craft the old state from a bunch of objects and arrays. You do not need to create instances of Deck, of Player, no mocking, no stepping through, none of that voodoo. In fact, this is already a test case. What's lacking is comparing previousState's values with newState's, seeing if deck was properly shuffled while nothing else changed.

const previousState = {
  phase: 'start', // start, in-game, end, etc.
  winner: null,
  deck: [ ...values... ],
  players: [
    { name: 'player1', hand: [/* empty hand */] },
    { name: 'player2', hand: [/* empty hand */] },
  ]
}

function shuffleDeck(state){
  return {
    // Copy everything over
    ...state,
    // But override deck
    deck: state.deck.reduce((shuffled, card) => {
      // For each card in the previous state's deck, insert them in random
      // positions in the array and return that array.
      shuffled.splice(Math.floor(Math.random() * shuffled.length), 0, card);
      return shuffled;
    }, []);
  }
}

const newState = shuffleDeck(state);

As you can see, shuffling the deck is a new state constructed from the old state, with the deck modified. From a testability point of view, you can easily craft the old state from a bunch of objects and arrays, feed it to the function and you get your result. You do not need to create instances of Deck, of Player, no mocking, no stepping through, none of that voodoo. In fact, this is already a test case. What's lacking is the assertions.

1
source | link

The one thing I find hard to deal with when it comes to OOP is how tied up the logic is to objects, and objects are to objects. For instance, why does the Deck class aware about the players. Shouldn't it just be dealing with cards?

Another problem with this is testability. Say you encountered a bug mid-game and wanted to reproduce it. How do you go about extracting that information from the game? How do you reach that state where the bug occurred? How do you test this code?

Card games are like state machines. They have a definite state at any point in time, and actions players take mutate that state. You can simply define the state structure like so:

const game = {
  phase: 'start', // start, in-game, end, etc.
  winner: null,
  deck: [],
  players: [
    { name: 'player1', wonDeck: [], warDeck: [] },
    { name: 'player2', wonDeck: [], warDeck: [] },
  ]
}

From here, you can tell from looking at the data what cards are in the deck, how many players there are, who are the players, what cards they have. You can easily serialize this data structure, use it for logging, debugging, reporting.

Now let's take a state transition. For instance, shuffling the deck would be like:

const previousState = {
  phase: 'start', // start, in-game, end, etc.
  winner: null,
  deck: [ ...values... ],
  players: [
    { name: 'player1', hand: [/* empty hand */] },
    { name: 'player2', hand: [/* empty hand */] },
  ]
}

function shuffleDeck(state){
  return {
    // Copy everything over
    ...state,
    // But override deck
    deck: state.deck.reduce((shuffled, card) => {
      // For each card in the previous state's deck, insert them in random
      // positions in the array and return that array.
      shuffled.splice(Math.floor(Math.random() * shuffled.length),card);
      return shuffled;
    }, []);
  }
}

const newState = shuffleDeck(state);

As you can see, shuffling the deck is a new state constructed from the old state, with the deck modified. From a testability point of view, you can easily craft the old state from a bunch of objects and arrays. You do not need to create instances of Deck, of Player, no mocking, no stepping through, none of that voodoo. In fact, this is already a test case. What's lacking is comparing previousState's values with newState's, seeing if deck was properly shuffled while nothing else changed.

Given this advice, the game would progress like:

  1. Generate initial state (a blank state like the first snippet)
  2. Wait for user input
  3. Mutate state (the second snippet)
  4. Render results of 3
  5. Repeat steps 2-4 until winner found

As for the rest of the code, there's... a few problems:

Your domain object (Player) and game logic should not be aware of the DOM. In MVC, only logic stays in the controller. Rendering is done in a view layer which should only receive already-processed data.

dealCards.onclick = function(){
  GameLogic(Player1, Player2);
}

Use addEventListener. Although onclick works, you're technically overriding the onclick property. Any previously added handlers would be replaced.

console.log(this.name +" loses war");

You should remove debugging code. Although it's understandable that this is a small app, it's still a bad habit. I've seen people years into web development that still make this simple mistake.

if(this.currentDeck.length < 2){
    if(this.wonDeck.length < 2){
        if((this.currentDeck.length === 1) && (this.wonDeck.length === 1)){

You have a lot of "magic numbers" in your code. What does 2 signify? What does 1 signify? Can I change this to 42? Why? Why not?. It is advised that you store these in properly named variables, named after their purpose.


Also you mentioned of a "potential employer" in the earlier revisions. If this employer is what I think it is, they'll be looking at code structure, maintainability and testability. These things you should look out for in your code.

Also, they'd want to reduce the time it takes to develop such apps. This starts with not repeating what other developers have already created. Consider venturing into using a framework, any will do. This will take out the hassle of DOM manipulation so that all you need to worry about is your data and logic. Vue.js is a good candidate for small and easy to start with.