Open
Conversation
HIPPIEKICK
approved these changes
Sep 12, 2024
Contributor
HIPPIEKICK
left a comment
HIPPIEKICK
left a comment
There was a problem hiding this comment.
Hey there! Awesome work on putting together this project 🥳
JavaScript
- In your code, you declare the variable
orderedFoodwithout using let or const. It's important to declare variables explicitly, as this avoids potential issues with global variables. - The flow is overall well controlled, you've added a lot of fallbacks for different alternatives which is great! However, maybe you'd want to consider to stop the flow when an invalid input is detected. You can use
returnto exit the function if the choice is invalid. For example:
if (pizzachoice !== "1" && pizzachoice !== "2" && pizzachoice !== "3") {
alert("Sorry, we don't have that kind of pizza.");
return; // Stop further execution
}
Clean Code
- See comments in code about indentation. Check your whole file
- Remember to always write variable names in camelCase (mealChoice, pizzaChoice)
Keep up the good work! Remember to think about these things going forward, and your code will be more readable and robust.
| After a while, I figured it must be because I was using digits, not text. I tested”, ’ and ´ until I spotted that a backtick/grave/grave accent was used in the first alert that came with the fork. Changed all string/text values to that specific tick and just like that, the code worked. This also made me discover that everything between two quotation marks turns into text, while the ${name} still function when using the backtick. So I used ”” for the meal choices (since it has to be only text) and for the alert, I used ` since it has ${name} in it. | ||
|
|
||
| ### Old variable? | ||
| ${name} turned into a strikethrough text. When I hovered over it, it said ”'name' is deprecated.” Googled it, and read that the warning is being shown because the name variable may be removed in future versions. Not sure if it affects my code, but to avoid any issues I changed it to $userName and the strikethough disappeared. |
Contributor
There was a problem hiding this comment.
name is an old JavaScript keyword, but changing to userName sounds like a really good solution ⭐
| alert(”Thanks, ${name}! You wish to order Pizza!”) | ||
| } | ||
|
|
||
| After a while, I figured it must be because I was using digits, not text. I tested”, ’ and ´ until I spotted that a backtick/grave/grave accent was used in the first alert that came with the fork. Changed all string/text values to that specific tick and just like that, the code worked. This also made me discover that everything between two quotation marks turns into text, while the ${name} still function when using the backtick. So I used ”” for the meal choices (since it has to be only text) and for the alert, I used ` since it has ${name} in it. |
code/script.js
Outdated
| ` | ||
| ) | ||
| //Pizza | ||
| if (mealchoice === "1") { |
Contributor
There was a problem hiding this comment.
I saw you mentioned this in your README as well, regarding indentation. If you've turned off the default formatting, make sure to have another look through your file and indent everything properly, e.g.
if (mealchoice === "1") {
const pizzachoice = prompt(
`Thanks, ${userName}! You wish to order Pizza!
What type of Pizza you like to order? Please enter the number of your choice.
1. Capricciosa
2. Hawaii
3. Vesuvius
`)
if (pizzachoice === "1") {
orderedFood = "Capricciosa pizza"
} else if (pizzachoice === "2") {
orderedFood = "Hawaii pizza"
} else if (pizzachoice === "3") {
orderedFood = "Vesuvius pizza"
} else {
alert("Sorry, we don't have that kind of pizza.")
}
if (orderedFood) {
alert(`You've chosen ${orderedFood}`)
}
} else if....
code/script.js
Outdated
|
|
||
| // Step 2 - Food choice | ||
| // Your code goes here | ||
| const mealchoice = prompt( |
Contributor
There was a problem hiding this comment.
camelCase! -> mealChoice
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Netlify link
Add your Netlify link here like this (update with the correct one):
https://johannas-js-pizzeria.netlify.app/