What is missing on this JS code to make it looping

Question

What is missing on this basic code to make it looping when the prompt input give error?

var userChoice = prompt("Do you choose rock, paper or scissors?");
var error = "Error. You should input rock, paper or scissors!";
var inputAgain = "Do you choose rock, paper or scissors?";
    if(userChoice!=="rock") {
        if(userChoice!=="paper") {
            if(userChoice!=="scissors") {
                confirm(error);
                userChoice = prompt("Do you choose rock, paper or scissors?");
            }
        }
    }

You need to add an outer loop so that until you meet some success criterion, it will keep looping. — RobG, Sep 12 '14 at 00:39
@JaredFarrish are you suggesting the two prompts with the same message have the appearance of a loop? — vol7ron, Sep 12 '14 at 01:09

volodymyr · Answer 1 · 2014-09-12T00:51:16.443

1

var requiredValues = ["rock","paper","scissors"];
while(requiredValues.indexOf(prompt("Do you choose rock, paper or scissors?"))<0){
    alert("Error. You should input rock, paper or scissors!");
}

edited Sep 12 '14 at 00:51

answered Sep 12 '14 at 00:46

volodymyr

11
2

This is like the worst code a person could write... Not human-readable, creating a new array instances every time, not even comments on there... – Derek 朕會功夫 Sep 12 '14 at 00:47
1

@Derek朕會功夫 This code (written when it was the previous code) isn't that bad, although I would separate the prompt from the condition for readability. Also, "creating a new array" is entirely irrelevant in terms of any performance implication here. – user2864740 Sep 12 '14 at 00:48
There's some poor choices made, but this doesn't even crack the Top 50 worst code examples I've seen in some answers on this site. – Jared Farrish Sep 12 '14 at 00:53
2

if you assign prompt(...) to a variable, then the result we be able to be used after the loop... ie `requiredValues.indexOf(userChoice = prompt("Do you choose rock, paper or scissors?"))` – br3nt Sep 12 '14 at 01:00
Agreed with Jared, in fact this is rather concise, which improves readability. Compare this 3-liner with brunt's answer and you'll know what's going on in a quarter of the time here. Plus, by making the possible values in an array, it adds scalability for when more values are needed (e.g., `bomb`, `ray gun`,`atomic bomb`). I'll add that br3nt is adding more, but he's also right above, assign the result. – vol7ron Sep 12 '14 at 01:00
Can't use this because i haven't learned the 'while' yet but i need to do exercises. – Alexandre Faustino Sep 12 '14 at 08:49

br3nt · Answer 2 · 2014-09-12T02:47:25.970

0

Your code needs a loop so that the question can continue to be asked and answered.

var options = ["rock","paper","scissors"],
    userChoice;

while (true) {
    userChoice = prompt("Do you choose rock, paper or scissors?");

    // stop asking if answer is valid
    if (options.indexOf(userChoice.trim().toLowerCase()) >= 0) break;

    // otherwise, inform user of error and continue asking
    confirm("Error. You should input rock, paper or scissors!");
}

edited Sep 12 '14 at 02:47

answered Sep 12 '14 at 00:52

br3nt

9,017
3
42
63

not a big fan of `do..while` would rather see this just as a `while`, especially since you've initialized ask_question to a value that will make the loop perform at least once – vol7ron Sep 12 '14 at 01:04
1

I'd also add a `toLowerCase()` and `trim()` as minimum precaution to reduce user error – vol7ron Sep 12 '14 at 01:07
ditto... lol. Though the purpose of `do..while` is to loop at least once. In this instance we want to ask rock, paper, scissors at least once. – br3nt Sep 12 '14 at 01:08
I would also extract our the conditional into a separate method. – br3nt Sep 12 '14 at 01:08
still saying that because your initial condition is always going to evaluate to `true`, you don't need the extra markup of the `do`, just place `while` at the top – vol7ron Sep 12 '14 at 01:13
Using *true..break* is very clunky, you should have an actual condition that is tested, e.g. something like `while (!(/rock|paper|scissors/i.test(userChoice))) {...}`. – RobG Sep 12 '14 at 02:08
@RobG, It's rather subjective. Check out [this accepted answer](http://stackoverflow.com/a/6850411/848668). If you look through my edits, you'll see I've done exactly what that answer suggests. The whole thread has some good comments for both sides of the argument. If anything, I should get rid of the `if..else`, in my opinion. Infact, I'll do that... lol – br3nt Sep 12 '14 at 02:25
There... next thing would be to factor out the input validation into a separate method. `if (user_input_valid(userChoice)) break;` – br3nt Sep 12 '14 at 02:27

easywaru · Answer 3 · 2014-09-12T02:21:55.283

Try this

var funcAskUserChoice=function(){
    var userChoice = prompt("Do you choose rock, paper or scissors?");

    // click prompt cancel
    if(userChoice === null){
        return;
    }

    // input is correct
    if(userChoice === "rock" || userChoice === "paper" || userChoice === "scissors"){
        return;
    }

    // click confirm cancel
    if(confirm("Error. You should input rock, paper or scissors!") === false){
        return;
    }

    // ask again
    funcAskUserChoice();
}

// start loop
funcAskUserChoice();

** DEMO **

score 0 · Accepted Answer · answered Sep 12 '14 at 02:24

I'm surprised the compiler did not detect the Again part of the variable name inputAgain and figure out that you wanted it to do a loop.

Anyway, recursion is often a good way to loop!

function keep_asking() {
    var userChoice = prompt("Do you choose rock, paper or scissors?");
    var error = "Error. You should input rock, paper or scissors!";
    if(userChoice!=="rock") {
        if(userChoice!=="paper") {
            if(userChoice!=="scissors") {
                confirm(error);
                return keep_asking();
            }
        }
    }
    return userChoice;
]

In ES6 there is tail recursion, so doing things this way is "free".

What is missing on this JS code to make it looping

4 Answers4