2
\$\begingroup\$

I have built a simple JS class with functional methods that create a Rock, Paper, Scissors game.

I would be happy to get a non-opinionated review in terms of code being optimal.

class Rps{
  constructor(){
    this.signs = {1:'rock',2:'paper',3:'scissors'};
    this.winMatrix = [
      {1:3},
      {2:1},
      {3:2}
    ];
  }
  randomizePlay(){
    const keys = Object.keys(this.signs);
    return 1 + Math.floor(keys.length * Math.random());
  }

  play(){
    return this.roundResult([this.randomizePlay(),this.randomizePlay()]);
  }

  roundResult(resultArray) {
    let victorious = {};

    if (resultArray[0] === resultArray[1]) {
      return 'rematch';
    }

    this.winMatrix.forEach((value, index) => {
      const p1Result = resultArray[0];
      const p2Result = resultArray[1];
      const p1WinMatrix = parseInt(Object.keys(this.winMatrix[index]));
      const p2WinMatrix =  parseInt(Object.values(this.winMatrix[index]));

      if (p1Result === p1WinMatrix && p2Result === p2WinMatrix){
        return victorious = {1:p1Result}
      }else{
        return victorious = {2:p2Result}
      }
    });
    return victorious;
  }
}

const theRpsArray = [];
const theRpsAverage = 0;

let rps = new Rps;
rps.play()
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Simulate only the result

A common problem in learning to program is over complicating the code. Complicated code is longer, harder to understand, and more prone to bugs. When the app is large even tiny over complications can lead to thousands of unneeded lines that must be tested and debugged.

All that matters is what comes out the other end of a function, how that is done does not matter.

Your code is way too complicated for what it needs to do. At its most simplest Rock paper scissors can be one line...

(["Tie ","P1 WINS with ","P2 WINS with "])[Math.random()*3|0]+ ((["Rock","Paper","Scissors"])[Math.random()*3|0])

Rewrite

The object rps is equivalent to yours. Calling rps.play() returns the same results, and is statistically identical to your results

const rps = (() => {
    const rand3 = () => Math.random() * 3 | 0;
    const plays = [
        () => "rematch", 
        () => ({"1": rand3() + 1}), 
        () => ({"2": rand3() + 1})
    ];
    return { play() { return plays[rand3()]() } };
})();

You do not need to simulate the game mechanics, just the game results.

There are 9 possible results.

  • 1/3rd are draws,
  • 1/3rd player 1 wins and
  • 1/3rd player 2 wins.

When a player wins there is a 1 in 3 chance for any move type.

Thus a result first picks a random winner or draw, if there is a winner assign that player a random hand. There is no need calculate the hand the winner beats as that is inferred by the result.

Example 2

The following example produces a more readable result as an Example

const rps = (() => {
    const moves = ["Rock", "Paper", "Scissors"];
    const rand = (range = 3) => Math.random() * range | 0;
    const win = (m = rand()) => `Player ${rand(2) + 1} ${moves[m]} beats ${moves[(m + 2) % 3]}`;
    const plays = [win, win, (m = moves[rand()]) => `${m} draws with ${m}`];
    return { play() { return plays[rand()]() } };
})();

var games = 20;
while (games --) { log(rps.play()) }



function log(textContent) {
    info.appendChild(Object.assign(document.createElement("div"),{textContent}));
}
<code id="info"></code>

\$\endgroup\$

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.