HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinorCanonical

Simple number theory game, pt. 3

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theorynumbergamesimple

Problem

More refactoring based you guys' great feedback. This time around I worked on using global/local variables properly and efficiently, putting return values to use, renaming variables so that the code is more understandable, and simplifying functions so that they serve one main purpose.

My main concerns/focuses now: I'm not sure that I like the global variable newScoreAlertIndex and how it's being implemented, and am working on how to better write the newScoreAlert function. Also, since the while loop can essentially be divided up into two structurally equivalent parts (the user's turn and the computer's turn), I was debating creating a function to run a full turn, and call it twice in the while loop, switching between user and computer factor/score parameters. However I felt like this blurred the line between query and command, so I left it as is. Let me know what you think.



`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 compScore = 2,
userScore = 2,
operator,
newScoreAlertIndex = 1;

function getFactors(in

Solution

A few suggestions to make your code more readable/succinct:

  1. Why not store your alerts in a single array?



var alerts = [alert("....",
              alert("....",...`


This makes your code more maintainable and your variable names less arbitrary. We have to agree rules2 is not such a hot name for a variable.

Note I don't know if the syntax I have above is correct. You might need to use bind or apply or something so the alert doesn't execute right away, but you can easily check this with a fast experiment...

Note in comments below SirPython has clarified that storing them this way will immediately run them. Consider his suggestion of storing the messages in an array rather than an alert function itself.

  1. getFactors does a lot of unnecessary checks



It's well known that to get the factors of a number, you only need to check numbers less than its square root. That's, by definition, the largest factor a number can have other than itself. So you can adjust the for loop like so:

check_lim = Math.floor(sqrt(integer)) + 1   
for(var i = 1; i< check_lim; i++) { 
   if(integer % i == 0){
       factors.push(i);
       factors.push(integer/i);


Relatedly, I'd also suggest changing integer to something that sounds less like a key/reserved word. Makes me nervous!

Also, now your factors won't be in order from smallest to greatest. If an ordered factor list is necessary for your code, and if you really are just sticking with small numbers, your present method could be better.

  1. Keep control statements as simple as possible



I'd rewrite checkForWin like so:

function checkForWin(number) {
    return ( number === 1  || number === 0 );
}

Code Snippets

var alerts = [alert("....",
              alert("....",...`
check_lim = Math.floor(sqrt(integer)) + 1   
for(var i = 1; i< check_lim; i++) { 
   if(integer % i == 0){
       factors.push(i);
       factors.push(integer/i);
function checkForWin(number) {
    return ( number === 1  || number === 0 );
}

Context

StackExchange Code Review Q#102608, answer score: 6

Revisions (0)

No revisions yet.