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

Feedback #1

Closed
wants to merge 18 commits into from
Closed

Feedback #1

wants to merge 18 commits into from

Conversation

github-classroom[bot]
Copy link

@github-classroom github-classroom bot commented Sep 8, 2021

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.

In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.

Click the Files changed or Commits tab to see all of the changes pushed to main since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

  • Click the Files changed tab to see all of the changes pushed to main since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
  • Click the Commits tab to see the commits pushed to main. Click a commit to see specific changes.
  • If you turned on autograding, then click the Checks tab to see the results.
  • This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.

For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @born3am

@born3am
Copy link
Collaborator

born3am commented Sep 9, 2021

Helon, my prompt sync console is running again when I am typing (before I press enter).

I also would like to request to only accept numbers, but the code below is not working:

while (!Number.isNaN(a)) {
a = prompt(Choose a NUMBER to be your "x" variable: );
}

@Lvrks
Copy link

Lvrks commented Sep 9, 2021

Helon, my prompt sync console is running again when I am typing (before I press enter).

I also would like to request to only accept numbers, but the code below is not working:

while (!Number.isNaN(a)) {
a = prompt(Choose a NUMBER to be your "x" variable: );
}

I think an if statement may make more since here. We only want to accept a if it is a number but we also can't pass a in as a parameter if it doesn't exist yet. So you may want to get a via a prompt then check if it is a number before proceed with your program.

You should also declare a with either let or const depending how you plant to use it. If you omit let, const, or var then, in simple terms, it's going to be accessible everywhere which can result in some hard to debug issues.

Will clone and look at the rest in a bit!

@born3am
Copy link
Collaborator

born3am commented Sep 9, 2021

the variables are declared at the top.

i will try to figure out regarding the "if is a number"

what about the other question? when you run the program there, does the menu jumps twice while you are typing too?

@Lvrks
Copy link

Lvrks commented Sep 9, 2021

Ah, I see - responded to your comment before I looked at the file.

It sounds like a bug, per one of this issue:
heapwolf/prompt-sync#25
and you also may want to make sure you don't have any prompt that ends with a \n:
heapwolf/prompt-sync#44

@born3am
Copy link
Collaborator

born3am commented Sep 13, 2021

Ah, I see - responded to your comment before I looked at the file.

It sounds like a bug, per one of this issue:
heapwolf/prompt-sync#25
and you also may want to make sure you don't have any prompt that ends with a \n:
heapwolf/prompt-sync#44

Thanks a lot!

How do you find these answers so easy? I spent hours trying to find the issue. How do you do your searches?

@Lvrks
Copy link

Lvrks commented Sep 13, 2021

I'd say it's just breaking down the issue and tracing it back to whatever may be causing it. Then, for googling it, all that really matters is the key words.

Since we didn't write the code for getting the prompts, I'd guessed it was most likely something to do with prompt-sync. From there, I narrowed it down to potentially coming from the \n character so I googled prompt-sync new line and slack overflow questions/github issues about the bug were the first thing to pop up.

I think that's why it also helps run your program often as well. You might be able to more easily catch the issue. Like, "oh, I just added this \n and now I have this issue so maybe that's what's causing it".

@born3am
Copy link
Collaborator

born3am commented Sep 14, 2021

Could you check my last commit (from today).

Everything works, but if I have to use my function validation1 or validation2 at least once, the number is not being recorded to the variables (variable MENU, xValue or yValue...)

@Lvrks
Copy link

Lvrks commented Sep 14, 2021

You're not actually validating before you make assignments and/or carry on with your program. For instance, If I try to make number x the letter d it will tell me that it's not a number and to choose another one but it's already assigned x the value of d. So you need to rethink your validation. Such as, if this is true then I will allow this, or I will allow this program to carry on. Right now, it looks like you're just calling the validation function but you're just alerting the user of the error.

@born3am
Copy link
Collaborator

born3am commented Sep 14, 2021

Yes. This is exactly my problem.

I am stuck since this morning.

I cannot find the solution following your instructions.

could you share a weblink with an example?

I did some progress (likely not the way you advised). REPO just PUSHED

But with the MENU the problem still remains.

@Lvrks
Copy link

Lvrks commented Sep 14, 2021

I think this article might help you https://medium.com/javascript-inside/effective-data-validation-in-javascript-5c2f3e75249e

It discusses more about the topic than perhaps necessary but I think still interesting. Still, you can see how they've written functions to validate data.

So the main point with our validation is that we don't want to allow these inputs - i.e. assign them to a variable - unless they pass our validation. It may look something like this:
userInput = validInput(userInput)
Where valid input is our function to validate the user input.

@born3am
Copy link
Collaborator

born3am commented Sep 14, 2021

ok. thanks!

@born3am born3am closed this Feb 1, 2024
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.

3 participants