Why doesn't this loop the menu?

Question

I'm using GNU bash, version 4.3.46 on a Ubuntu machine. For some reason this while loop doesn't work as expected.

The menu should loop continuously until the user decides to quit the program, then there is some error checking prompting the user if they are sure, then the program ends.

Here's the code:

#!/bin/bash

menu_choice=0
quit_program="n"

while [[ $menu_choice -ne 3 && $quit_program != "y" ]]
do

    printf "1. Backup\n"
    printf "2. Display\n"
    printf "3. Exit\n\n"

    printf "Enter choice: \n"
    read menu_choice

    if [ $menu_choice -eq 3 ]
    then
        printf "Are you sure you want to quit? (y/n)\n"
        read quit_program

    fi

done

I think it might have to with global variables are declared at the beginning and I'm reading in a new value locally...

You say it doesn't work as expected, but what is the actual behavior? — TheWanderer, Mar 26 '17 at 18:35
Hi @Zacharee1 the menu loops when the users enters an integer 1 or 2, but it fails to loop under option 3 when the user is prompted if they want to quit. The program should continue to loop even if they type "n" but the program exits. — Mondo192, Mar 26 '17 at 18:38
That's because you're using && for the conditions in the while loop. You want it to loop as long as either is true, not both at once. Try using || instead. — TheWanderer, Mar 26 '17 at 18:46
Regardless I'd suggest considering using a select... case construct instead: see How can I create a select menu in a shell script? — steeldriver, Mar 26 '17 at 18:52
No need to check the menu_choice variable at all for the while loop. See my answer. — Delorean, Mar 26 '17 at 19:08

score 7 · Accepted Answer · answered Mar 26 '17 at 18:51

7

The problem is in your while loop conditions. This line:

while [[ $menu_choice -ne 3 && $quit_program != "y" ]]

is saying "while menu_choice isn't 3 and quit_program isn't y, keep looping." The problem is that, if either of those conditions is no longer true, the while loop will end.

What you want is this:

while [[ $menu_choice -ne 3 || $quit_program != "y" ]]

using || instead of &&. This way, the while loop will continue to loop as long as either condition is true, instead of both.

answered Mar 26 '17 at 18:51

TheWanderer

19,395
12
50
65

1

Checking the menu_choice variable is an unnecessary check for the while loop. It is not needed for the loop at all and overcomplicates the code for nothing. See my answer. – Delorean Mar 26 '17 at 19:06
@Dorian you didn't downvote this, did you? – TheWanderer Mar 26 '17 at 19:47
I don't downvote other people's answers unless they are completely wrong or way off base. – Delorean Mar 26 '17 at 19:59

Delorean · Answer 2 · 2017-03-26T19:02:52.540

This simpler script should work for you

#!/bin/bash

menu_choice=0
quit_program=false

while [ $quit_program == false ]
do

    printf "1. Backup\n"
    printf "2. Display\n"
    printf "3. Exit\n\n"

    printf "Enter choice: \n"
    read menu_choice

    if [ $menu_choice -eq 3 ]
    then
        printf "Are you sure you want to quit? (y/n) "
        read ask
        if [ $ask == "y" ]
        then
            quit_program=true
        fi

    fi

done

printf "\nDone\n"

There's no need to keep checking the menu_choice, so that can be removed from the while loop check.

In my example above, I just set a boolean of quit_program which is checked in the loop. If the user chooses option 3, and then says "y" to the confirmation, then the boolean is set to true to kill the loop.

You can also go even further without checking a boolean with this:

#!/bin/bash

menu_choice=0

while true
do
    printf "1. Backup\n"
    printf "2. Display\n"
    printf "3. Exit\n\n"

    printf "Enter choice: \n"
    read menu_choice

    if [ $menu_choice -eq 3 ]
    then
        printf "Are you sure you want to quit? (y/n) "
        read ask
        if [ $ask == "y" ]; then break; fi
    fi
done

printf "\nDone\n"

This second example accomplishes the same thing, but the while loop just runs without checking the boolean from before. The loop is broken with the break command instead.

Why doesn't this loop the menu?

2 Answers2