10
\$\begingroup\$

I just started JavaScript about a week ago. For my first project, my goal was to use my knowledge of basic JavaScript to create somewhat of a "thinking computer brain," so to speak. I wasn't focused on the looks of it, as seen by my only using the basic prompts and alerts. I just wanted to get used to writing coherently in JavaScript.

The game "works" but I feel that the code is overly verbose and could be much more succinct. But since I'm new to JS, I'm having trouble figuring out how to go about it. I understand if this is asking too much, but maybe some of the math people might find the game at least a little interesting.

var welcome = alert("Welcome to 2-to-1. Both you and the computer will start the game off with the number '2'. Whoever gets to '1' first wins! But of course, there's a catch.");

var rules = alert("This is a turn based game. You must first choose a factor of the computer's number. This can be any factor EXCEPT the number 1. If the chosen factor is even, it is added to your current score; if odd, it is subtracted. After this, the computer chooses a factor from your new number. This back-and-forth continues until someone gets their number down to 1.");

var rules2 = alert("A few extra rules: When an odd number is being subtracted from your score, if your resulting score is a negative number, the absolute value is taken, turning the negative number into a positive. Also, if your score ever reaches 0, this results in an automatic loss and the game ends.");

var rules3 = alert('Lastly, instead of choosing a factor from the computers score, you can choose to simply add 1 to your current score. To do this, type "+1" without the quotes. Good Luck!');



var yourDiv;
var compDiv;
var operator;
var yourNumber = 2;
var compNumber = 2;

function getFactors(integer) {
    var factors = [],
    quotient = 0;

    for(var i = 1; i <= integer; i++){
        quotient = integer/i;

        if(quotient === Math.floor(quotient)){
            factors.push(i); 
        }
    }
    factors.shift();
    return factors;
}


while (compNumber !== 1 && yourNumber !== 1) {

    var initCompFactors = getFactors(compNumber);
    var idk = 1;

    yourDiv = prompt("Your score: " + yourNumber + ". Computer's score: " + compNumber +  ". Which factor (besides 1) do you choose from the computer's score?");

    for (var i = 0; i <= initCompFactors.length - 1; i += 1) {

        if (parseInt(yourDiv) === initCompFactors[i]) {
            idk += 1;
        }
    }

    if (yourDiv === "+1") {
        idk += 1;
    }

    while (idk !== 2) {
        alert("Not a valid entry.");
        yourDiv = prompt("Your score: " + yourNumber + ". Computer's score: " + compNumber +  ". Which factor (besides 1) do you choose from the computer's score?");

        for (var i = 0; i <= initCompFactors.length - 1; i += 1) {

            if (parseInt(yourDiv) === initCompFactors[i]) {
                idk += 1;
            }
        }

        if (yourDiv === "+1") {
            idk += 1;
        }
    }

    if (yourDiv === "+1") {
        yourNumber += 1;
        yourDiv = "1";
        operator = "added to";
    } else {

        if (parseInt(yourDiv) % 2 === 0) {
            yourNumber += parseInt(yourDiv);
            operator = "added to";
        } else {
            yourNumber -= parseInt(yourDiv);
            operator = "subtracted from";
        }
    }
    if (yourNumber < 0) {
        yourNumber = yourNumber * -1;
    }

    var yourResult = alert(yourDiv + " was " + operator + " your score. You now have " + yourNumber + ".");

    if (yourNumber === 0) {
        alert("Sorry! You lost :/");
        break;
    }
    if (yourNumber === 1) {
        alert("YAY! You won!");
        break;
    }

    var yourFactors = getFactors(yourNumber);
    var doNotUse = [];


        for (var i = 0; i <= yourFactors.length - 1; i += 1) {
            if (yourFactors[i] % 2 !== 0) {
                if (compNumber - yourFactors[i] === 1 || compNumber - yourFactors[i] === -1) {
                    compDiv = yourFactors[i];
                    compNumber = 1;
                }
            }
        }

        if (compNumber !== 1) {



            compDiv = yourFactors[Math.floor(Math.random() * yourFactors.length)];

            if (yourFactors.length !== 1) {

                while (compNumber % 2 !== 0 && compDiv === compNumber) {
                    compDiv = yourFactors[Math.floor(Math.random() * yourFactors.length)];
                }

                for (var i = 0; i <= yourFactors.length - 1; i += 1) {

                    if (yourFactors[i] % 2 !== 0) {

                        var rawr = compNumber - yourFactors[i];
                        var compFactors = getFactors(rawr);

                        for (var j = 0; j <= compFactors.length - 1; j += 1) {

                            var compFactor = compFactors[j];
                            if (compFactor % 2 !== 0) {
                                if (yourNumber - compFactor === 1 || yourNumber - compFactor === -1) {

                                    if (doNotUse.indexOf(yourFactors[i]) === -1) {

                                        doNotUse.push(yourFactors[i]);
                                    }
                                }
                            }
                        }
                    }

                    if (yourFactors[i] % 2 === 0) {

                        var rawr = compNumber + yourFactors[i]; // 2 + 4 = 6
                        var compFactors = getFactors(rawr); // [2,3,6]

                        for (var j = 0; j <= compFactors.length - 1; j += 1) {

                            var compFactor = compFactors[j];
                            if (compFactor % 2 !== 0) {
                                if (yourNumber - compFactor === 1 || yourNumber - compFactor === -1) {

                                    if (doNotUse.indexOf(yourFactors[i]) === -1) {

                                        doNotUse.push(yourFactors[i]);
                                    }
                                }
                            }
                        }
                    }
                }
                console.log(doNotUse);

                if (doNotUse.length !== yourFactors.length) {
                    while (doNotUse.indexOf(compDiv) !== -1) {

                        compDiv = yourFactors[Math.floor(Math.random() * yourFactors.length)];

                    }
                }
            }

            if (compDiv % 2 === 0) {
                compNumber += compDiv;
            } else {
                compNumber -= compDiv;
            }
            if (compNumber < 0) {
                compNumber = compNumber * -1;
            }
        }

    var compResult = alert("The computer chose the factor " + compDiv + " from your score of " + yourNumber + ". The computer's new score is " + compNumber + ".");

    if (compNumber === 0) {
        alert("YAY! You won!");
        break;
    }
    if (compNumber === 1) {
        alert("Sorry! You lost :/");
        break;
    }
}

\$\endgroup\$
6
\$\begingroup\$

You've written the program as a single function. The overall structure can be made more apparent and the code easier to follow if you define helper functions which encapsulate smaller steps of the full program.

For instance, if you take the 30,000 ft view of your program, it looks like this (in pseudo-code):

while (neither player has won) {
  get the player's number
  update the player's score
  check if player has won or lost
  determine the computer's number
  update the computer's score
  check if the computer has won or lost
}

Each of these steps can be put into their own helper function which are also called "subroutines". The will make the body of the while loop a lot more readable.

Here is an example of a subroutine to get a valid number from the player (again, pseudo-code):

function getPlayersDivisor(computersNumber) {
    while (true) {
        ...prompt for a number, put into var yourDiv...
        if (yourDiv is a divisor of computersNumber or is "+1") {
            return yourDiv;
        }
        alert("Not a valid entry");
    }
}

Note how the while loop avoids the duplication of code that you have in your program.

Here is another idea for a subroutine - one that updates a player's score and returns how to report it. Either the computer's or player's score can be updated with this:

function updateNumber(oldNumber, divisorChosen) {
    var newNumber;
    var message;
    if (divisorChosen == 1 || divisorChosen % 2 == 0) {
      newNumber = oldNumber + divisorChosen
      message = divisorChosen + " was added to";
    } else {
      newNumber = oldNumber - divisorChosen;
      if (newNumber < 0) {
        newNumber = - newNumber;
      }
      message = divisorChosen + " was subtracted from";
    }
    return [ newNumber, message ];  # returns multiple values
}

Finally, you should put all of the computer's decision logic in a separate function, e.g.:

function chooseForComputer(playerNumber) {
    # logic for choosing a divisor for the computer goes here
    ...
    return compDiv;
}

With these helper functions this is what the while loop could look like:

// Assume these variables are defined:
//    player  - the player's number
//    computer - the computer's number
//
// and that neither player has won.
while (true) {
    playersDiv = getPlayersDivisor(computer);
    var result = updateNumber(player, playersDiv);
    player = result[0];
    var message = result[1];
    alert(message + " your score. Your number is now " + player)

    if (player == 0) { alert ("You lost."); break }
    if (player == 1) { alert ("You won.!"); break }

    var computerDiv = chooseForComputer(player);
    var result = updateNumber(computer, computerDiv);
    computer = result[0]
    alert("Computer choose " + computerDiv +" and now has score " + computer);

    if (computer == 0) { alert("Computer lost."); break }
    if (computer == 1) { alert("Computer won!"); break }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Heh, weird. I was writing my own answer, and hadn't seen yours until I posted it. But they're extremely similar (high level view, pseudo-code steps, etc.). But I swear, no plagiarism intended :) \$\endgroup\$ – Flambino Aug 31 '15 at 21:09
  • 1
    \$\begingroup\$ Great minds think alike :-) \$\endgroup\$ – ErikR Aug 31 '15 at 22:36
  • \$\begingroup\$ Thank you so much for such awesome feedback/advice! As I stated in my comment to Flambino's answer, I'll be poring over this thread tomorrow when I start the refactoring process. Your insight has been extremely helpful :) I'll try to update my original post soon after I feel comfortable with the changes I make. I feel like the point of these comments are for me to inquire about your answer, but it was all very clear, I don't know what else to say besides thank you guys! \$\endgroup\$ – Chris Sep 1 '15 at 2:47
5
\$\begingroup\$

I'd start by breaking this into more manageable chunks. The nice thing about this game is that there's very little state to keep track of. Essentially it's just two numbers (the scores), and the rest is derived from that. That makes it easy to turn things into functions, rather than (very) big blocks of code.

For instance, to check if a number is valid, you really only need something like:

function isValidFactor(factor, score) {
  return factor > 1 && score % factor === 0;
}

Of course you'll first want to check if the user entered +1, but otherwise that single line takes care of input validation.

Of course, you might still want your getFactors function for when the computer has to guess, but it can be simplified:

function getFactors(integer) {
  var factors = [];

  for(var i = 2; i <= integer; i++){
    if(integer % i === 0){
      factors.push(i); 
    }
  }

  return factors;
}

Note that the modulo operator does the work; you don't need to divide and check the quotient against its floored self. You also can just start the loop at 2, instead of including 1 just to shift it off the array afterward.

You can also simplify the computer's guessing. Rather than test all the possible factors, you can start by testing just the ones that'd give the computer a win straight away: The computer's score, if it's even, plus/minus 1. If either of those are a factor of the player's number, the computer wins. No need to loop through a bunch of things. And you can use a similar strategy to avoid landing on numbers that'd give the user a win.

You'll also want to reduce repetition. For instance, in your current code, you very often have to pick a random element from an array. That's another prime candidate for being made into a function. For instance:

function sample(array) {
  var index = Math.floor(Math.random() * array.length);
  return array[index];
}

Now you can take expressions like compDiv = yourFactors[Math.floor(Math.random() * yourFactors.length)]; and simply write compDiv = sample(yourFactors);.

Next, overall structure: From a high-level perspective, the game goes like this:

  • User picks a number
  • If game over, stop and announce winner
  • Computer picks a number
  • If game over, stop and announce winner
  • Repeat

This lines up well with a couple of functions and a simpler structure. For instance,

while(true) {
  getUserNumber();
  if(isGameOver()) break;

  getComputerNumber();
  if(isGameOver()) break;
}

I haven't actually thought about how to write getUserNumber or getComputerNumber yet, but this serves as a starting point - a sketch, really. It will no doubt change, but it allows me to start working out the missing functions independently. They don't have to all fit in the same loop.

Obviously you can go in any direction you want with those functions, but again the point is you're working on separate pieces and don't need to juggle everything at once. The functions themselves can be broken down even further like with the validity check above; that's a separate function that'll be of use to whatever function prompts the user. Try to keep going until you have very simple functions that do one thing, and one thing only (e.g. check a number), and there's virtually no repetition. If something's repeated, make it a function.

One final note: Your variable names could use some improvement. For instance idk isn't really very descriptive. Same goes for doNotUse and rawr. I have to parse through the code myself to figure out what they mean. Not a lot of fun.

Basically, code is read many more times than it's written. So make it easy to read. I know it's not hard to keep track of everything yourself when you're writing it (if you're in the middle of it, you have everything in your head, and it all makes sense), but it's not doing you any favors.

\$\endgroup\$
  • 1
    \$\begingroup\$ I'm certainly glad I checked back in on this question before heading to bed! Wow, I certainly wasn't expecting such a detailed answer. I cannot thank you enough :) I briefly read over everything you said and it's all extremely helpful; I can't wait to revisit this more thoroughly tomorrow and put your advice to work. Again, for me being just a beginner with all of this, it seriously means a lot to receive such in-depth feedback/help. I'll be sure it doesn't go unused :) \$\endgroup\$ – Chris Sep 1 '15 at 2:31
  • \$\begingroup\$ @Chris After implementing the improvements post a new question, do no update this. \$\endgroup\$ – Caridorc Sep 1 '15 at 7:36
2
\$\begingroup\$

Not a complete answer, but when declaring multiple variables you can - and should - use only one var keyword. Instead of

var a;
var b;
var c = 4;
var d = "foo";

use

var a,
    b,
    c = 4,
    d = 10;
\$\endgroup\$
  • \$\begingroup\$ I am not following why you would do that, please explain. I have been told not to do this, because of weird scope functionality when you do it like this. \$\endgroup\$ – Malachi Aug 31 '15 at 19:37
  • 2
    \$\begingroup\$ No. The "weird scope functionality" only happens if you mess up and put a semicolon instead of a comma. This style makes your code less verbose, which is what you asked for help with. \$\endgroup\$ – Bluefire Aug 31 '15 at 20:04
  • \$\begingroup\$ Huh, I feel like I should have already known this! Thank you for the tip; I'll be sure to declare multiple variables in this manner from now on. :) \$\endgroup\$ – Chris Sep 1 '15 at 2:52

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.