Simpler number theory game

After receiving some awesome feedback in my first post yesterday, I was able to grasp the "building block" method of creating javascript in chunks and putting them together to create coherent/straightforward code.

I've done quite a bit of refactoring and am fairly pleased given my initial goals for this project, but I know that there is always room for improvement! So here's the updated code for anyone interested in taking a look. The main thing that still bugs me now is the newNumber variable. Whereas everything else is nice and tucked away in its own function, newNumber gets passed around in several different places. Hopefully that makes sense!

I suppose I should note that the game mechanics are a little different this time around. I realized that the previous iteration involved less strategy than I originally intended, so I mixed some things up. I also focused less on making the computer think a couple steps ahead, so the game will play more randomly now.



`var welcome = alert("Welcome to Zeros or Ones! Both you and the computer will start the game off with the number '2'. Whoever gets down to '1' or '0' first wins!"),

rules = alert("This is a turn based game. You must first choose a factor from the computer's number. 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 or 0."),

rules2 = alert("A few exceptions:\n\nWhen 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.\n\nNeither player is allowed to win the game on a 2. For instance if your score is 2, you cannot choose the factors 1 or 3 from the computer's score.\n\nException to the exception! In the case that you have a 2 and the computer has a 3, it is allowable to choose either factor.");

var compNumber =

Nice update, kudos. Still, I have some comments.

-
As mentioned in a comment: Check your indentation. As Caridorc says, 4 spaces of indentation (or 1 tab, rendered with a width of 4 spaces) is a common standard for JavaScript. I tend to use 2 spaces myself, but that's less prevalent. So check your editor's settings.

-
Global variables

You have a bunch of them - more than you need. And more than you want; generally you'll want to keep the number of global variables to a minimum.

An example can be found in updateScore. It uses a global operator variable, yet that variable doesn't need to be global. Its value is set and used all within updateScore, and thus it should be a local variable.

-
Return values

This is related to the above, but I notice you're rarely using a function's return value for anything. Your function both alters a global variable, and returns that same value; pick one or the other.

For instance, you have these three lines:

playerChoosesFactor();
updateScore(yourDiv, " your score. You now have ", yourNumber);
yourNumber = newNumber;

Looking at just this, the logic isn't clear. That is, it makes sense as text ("these are the steps..."), but less so as actual code: yourDiv and youNumber are just there, somehow. It's not clear they're being changed. It's also a little odd that even though playerChoosesFactor actually returns something, that return value is just ignored. Some other languages/compilers will actually complain if you don't do something with a return value, since that's probably a bug.

Generally you'll want to treat your functions as either "commands" or "queries". This isn't a hard and fast rule or anything, but it helps keep things clear. A command changes something (i.e. a global score value) but doesn't (need to) return anything, while a query merely returns something, but doesn't cause things outside its scope to change (i.e. return a list of factors for a given number). So for instance, playerChoosesFactor makes the most sense as a query-type function. Simply getting the user's choice doesn't change anything - nor should it.

On a side note, the mix between "player" and "you" to designate the user is a little confusion. Not terribly so, but in the interest of consistency, I'd stick to one or the other - or pick "user", since that's the usual opposite of "computer" ("player" could mean either one). I'd also suggest calling the score variables something with score in it rather than just number, i.e. userScore instead of yourNumber. There's also a consistency thing with "factor" vs. "div"; player chooses a factor, yet it's called a divisor. Sure, same deal, but that's all the more reason to call it the same.

Anyway, something slightly more straight forward, with regard to return values, would be:

var factor = getUserFactor(); // factor is a local variable, since it's only used for these two lines
updateUserScore(userFactor); // a "command" function, no return

I've left out the text you use to create the alert, but that's just for clarity.

There is a point, though, to keeping the logic and the user interface - the alerts in this case - separate. Right now, you're coupling them tightly although there's no need. You can alert the user without updating the score, or update a score without alerting anyone. Yes, it makes sense to do both at the same time, but it's not intrinsically linked, so making them so in code is not really called for.

StackExchange Code Review Q#102550, answer score: 3