41w / MI-449-js-conditionals

0 stars 0 forks source link

Project Feedback #1

Open 41w opened 5 years ago

41w commented 5 years ago

Write a short choose-your-own adventure game in JavaScript

@egillespie Can you take a look at this? It's hosted here and meets the following criteria:

egillespie commented 5 years ago

I like your phone picker game, @41w — especially the way you created a function that you can reuse in different parts of your code! 🤘

There are a couple of spots in the function where things aren't quite working as I think you intended, and the rest of your code looks and works great. Do you mind touching these up?

Use || to provide default values

I see here that you're trying to provide a default value when someone types an invalid value:

  var numberChoice = window.prompt('How many would you like to buy?')
  numberChoice = parseInt(numberChoice.trim()) && 1
  if (numberChoice >= 1) {
    window.alert('Great! I will order for you.')
  } else {
    window.alert('You seem not like it, maybe Samsung?')
  }

It's totally awesome that you're providing default values like this. This style of default value is used a lot in professional and open source web projects! 🤘

The code you have here won't quite work like one might think, though. If I type z, for example, I will see "You seem not like it" instead of "Great! I will order for you." This happens because the && operator will return the first falsy value in a condition, and since parseInt('z') is the same as NaN, it is falsy. 😕

The operator you're looking for is ||. The || will give us the first truthy value in a condition. So in this example:

numberChoice = parseInt('z') || 1

parseInt('z') is the same as NaN because 'z' is not a number, so it's falsy. Then the || tells JavaScript to try the next expression (1) and return it if it is truthy. 1 is a truthy value (it is something), so numberChoice will be set to 1.

Hopefully that makes sense. Truthiness and falsiness can seem a bit complicated at first. 😅

Do you mind updating your example to use || instead of &&?

Do not call .trim() when using parseInt()

In your furtherChoice function, you are calling .trim() on the numberChoice variable and then using the value in parseInt:

  var numberChoice = window.prompt('How many would you like to buy?')
  numberChoice = parseInt(numberChoice.trim()) && 1

The bad news is that if I click the Cancel button on the prompt, then calling numberChoice.trim() will cause an error and your code will stop running. This happens because clicking Cancel will set numberChoice to null, and .trim() is not a function of null. 😱

The good news is that parseInt will automatically ignore any spaces when figuring out which number was typed. So you can call parseInt(numberChoice) and get the same result no matter how many spaces I type in the prompt. It also means that if I click Cancel, the code will keep running. 😄

Do you mind removing the .trim() in your furtherChoice function?


After you’ve made your changes and pushed them to GitHub and your hosted site, give it a once-over to make sure it looks right, then comment back here and I’ll take another look.

Thanks! 📱

41w commented 5 years ago

Hi @egillespie I didn't change && to || because I want the window to prompt "You seem not like it" for all inputs other than numbers. If I want to do that, is my code working the right way? I have updated the others, can you take a look at my code? Thank you!

egillespie commented 5 years ago

Your changes look great! 🎉

I see what you're saying about leaving &&. With your explanation I think it makes sense and it looks like the code behaves that way in most cases.

If someone types "-3" in the prompt, what message do you want to show them? Right now I see "Great! I will order for you." if I type a negative number.

41w commented 5 years ago

@egillespie I have just updated my code, I didn't realize that -1 && 1 returns 1 instead of -1.

So I just use the if statement to check if the input is a number bigger or equal to 1, and all other results will go to the else statement. I hope this will be correct, can you take a look at it?

Thank you!

egillespie commented 5 years ago

Yeah, truthiness and falsiness can be pretty tricky. 😕

I like your latest change, though, and don't have any other recommendations for improvement. Nice work! 😃 :shipit: