Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xings JS Pizzeria #145

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Xings JS Pizzeria #145

wants to merge 9 commits into from

Conversation

xingyin2024
Copy link

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice job on this project Xing!

JavaScript

  • Nice to see that you're trying out both the if-else approach as well as the switch approach! However, for the future, the more consistent code, the more clean 😊
  • The variable naming (userName, selectedFood, etc.) is clear and descriptive, which makes the code readable.
  • The exit(1) is intended for server-side Node.js or environments that support it, but in the browser context, you could use return to exit the current function

Clean Code

  • Your well-named variables make the code easier to follow
  • Pay attention to consistent indentation and spacing (e.g. see comment in the code). Consistency makes your code more readable and professional.

Overall, you've done a really good job, so keep it up ⭐

@@ -2,18 +2,146 @@

// Step 1 - Welcome and introduction
// Your code goes here
alert(
`Welcome to our Javascript Pizzeria. Ready to Start? - Click 'OK' to begin.`
alert (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no space between JavaScript methods and their corresponding parentheses. Please go through your whole file and change where it applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants