217

I tried to check if the PHONE_TYPE variable contains one of three valid values.

if [ "$PHONE_TYPE" != "NORTEL" ] || [ "$PHONE_TYPE" != "NEC" ] ||
   [ "$PHONE_TYPE" != "CISCO" ]
then
    echo "Phone type must be nortel,cisco or nec"
    exit
fi

The above code did not work for me, so I tried this instead:

if [ "$PHONE_TYPE" == "NORTEL" ] || [ "$PHONE_TYPE" == "NEC" ] ||
   [ "$PHONE_TYPE" == "CISCO" ]
then
    :        # do nothing
else
    echo "Phone type must be nortel,cisco or nec"
    exit
fi

Are there cleaner ways for this type of task?

Improve this question

8 Answers 8

Reset to default
286

I guess you're looking for:

if [ "$PHONE_TYPE" != "NORTEL" ] && [ "$PHONE_TYPE" != "NEC" ] &&
   [ "$PHONE_TYPE" != "CISCO" ]

The rules for these equivalents are called De Morgan's laws and in your case meant:

not(A || B || C) => not(A) && not(B) && not (C)

Note the change in the boolean operator or and and.

Whereas you tried to do:

not(A || B || C) => not(A) || not(B) || not(C)

Which obviously doesn't work.

Improve this answer
62

If using bash, a much shorter way would be:

if [[ ! $PHONE_TYPE =~ ^(NORTEL|NEC|CISCO)$ ]]; then 
  echo "Phone type must be nortel, cisco or nec."
fi
  • ^ – To match a starting at the beginning of line
  • $ – To match end of the line
  • =~ - Bash's built-in regular expression comparison operator
Improve this answer
2
  • 3
    The question doesn't mention bash, and in fact deliberately only mentions shell script, both in the tags, and in the question. This answer would fail in a POSIX environment. Commented May 16, 2021 at 17:21
  • If you're using Bash (recommended) as this answer, there's an even cleaner syntax you can use, please see my answer below. Commented Apr 23, 2023 at 11:23
20

Good answers, and an invaluable lesson ;) Only want to supplement with a note.

What type of test one choose to use is highly dependent on code, structure, surroundings etc.

An alternative could be to use a switch or case statement as in:

case "$PHONE_TYPE" in
"NORTEL"|"NEC"|"CISCO")
    echo "OK"
    ;;
*)
    echo "Phone type must be nortel,cisco or nec"
    ;;
esac

As a second note you should be careful by using upper-case variable names. This is to prevent collision between variables introduced by the system, which almost always is all upper case. Thus $phone_type instead of $PHONE_TYPE.

Though that one is safe, if you have as habit using all upper case, one day you might say IFS="boo" and you're in a world of hurt.

It will also make it easier to spot which is what.

Not a have to but a would strongly consider.

It is also presumably a good candidate for a function. This mostly makes the code easier to read and maintain. E.g.:

valid_phone_type()
{
    case "$1" in
    "NORTEL"|"NEC")
        return 0;;
    *)
        echo "Model $1 is not supported"
        return 1;;
    esac
}

if ! valid_phone_type "$phone_type"; then
    echo "Bye."
    exit 1
fi
Improve this answer
2
  • +1 for use of case statement, avoiding the complexity of regular expression. Commented Dec 6, 2021 at 18:17
  • 1
    The double-quotes in case "$var" in ... are not necessary because word-splitting doesn't occur in that context. Commented Dec 6, 2021 at 18:18
14

You should use ANDs, not ORs.

if [ "$PHONE_TYPE" != "NORTEL" ] && [ "$PHONE_TYPE" != "NEC" ] && [ "$PHONE_TYPE" != "CISCO" ]
then

or 

if [ "$PHONE_TYPE" != "NORTEL" -a "$PHONE_TYPE" != "NEC" -a "$PHONE_TYPE" != "CISCO" ]
then
Improve this answer
1

To correct an above answer (as I can't comment yet):

PHONE_TYPE="NORTEL"
if [[ $PHONE_TYPE =~ ^(NORTEL|NEC|CISCO|SPACE TEL)$ ]]; then 
  echo "Phone type accepted."
else
  echo "Error! Phone type must be NORTEL, CISCO or NEC."
fi

Please note that you need at least bash 3.2 for this use of =~

I tested on MS Windows 7 using bash 4.3.46 (works fine) and bash 3.1.17 (didn't work).

=~ was introduced in 3.1 but in that version, you had to use:

[[ $PHONE_TYPE =~ '^(NORTEL|NEC|CISCO|SPACE TEL)$' ]]

In 3.2, the interface changed in that quoting operators such as '...' above would cause regexp operators (such as ^, (), |, $) to lose their special meaning¹, but space (which is otherwise special in the shell syntax if not the regexp syntax) could then be left unquoted if found within matching (...) pairs¹.

To do regexp matching portably across versions of bash 3.1+ and zsh, ksh93 and yash and avoid the bugs, best is to store the regexp in a variable:

phone_regexp='^(NORTEL|NEC|CISCO|SPACE TEL)$'
if [[ $PHONE_TYPE =~ $phone_regexp ]]; then...

¹ With some exceptions and bugs most of which have been fixed in the latest version.

Improve this answer
0

Less portable for POSIX, but works in Bash:

if [[ "${PHONE_TYPE}" != @(NORTEL|NEC|CISCO) ]]; then 
    echo 'Phone type must be NORTEL, CISCO, or NEC' >&2
    exit 1
fi

Edit: Yes, you should almost always use braces and quotation marks on your bash variables, which is why I reverted the edits to my answer.

https://nickjanetakis.com/blog/why-you-should-put-braces-around-your-variables-when-shell-scripting

Improve this answer
4
  • 1
    sure, you're free to use braces around variable names always, if you like. But I don't think that linked page makes a convincing argument about that. The guy is going over the cases where braces are required, and trying to them without the braces, even though some of them are rather obvious in that they cannot work (i.e. it should be clear that $personiscool is a single variable expansion, not an expansion plus a literal). He appears to have no arguments at all for bracing the simple cases, where braces are not required, such as the one here. Commented Sep 10, 2024 at 7:15
  • You're like the 25th person to say this, a lot of things come down to personal preference, readability and organized code. I have a large bash project and I'm not going to "sometimes" use brackets just because they aren't always required. Additionally, editing someone's answers when their code is valid and works is frivolous too. Commented Sep 10, 2024 at 11:35
  • 1
    yes, and as I said, you're free to code your project the way you like. But, next to stating your preference to using braces, you linked to an article titled "Why You Should Put Braces around Your Variables When Shell Scripting", but TBF, I don't see much rationale for always using braces in that article. I mean, I can see reasonable arguments to the lines of "it's more consistent" and "${foo}_$bar" looks inconsistent; or "that way you never accidentally forget them in a context that requires them", but I don't see those mentioned there. Commented Sep 10, 2024 at 12:16
  • 1
    And that's what I'm commenting on, bad arguments. Note that you're not just saying you want to do it that way, but saying the reader "should" too. That requires some reasoning: you don't get to just tell people to do things your way just like that. (Unless you're their boss, which for most SE users you probably aren't.) Commented Sep 10, 2024 at 12:26
-1

Just a variation proposal based on @0x80 solution:

# define phone brand list
phoneBrandList=" NORTEL NEC CISCO" ## separator is space with an extra space in first place

# test if user given phone is contained in the list
if [[ ${phoneBrandList} =~ (^|[[:space:]])"${userPhoneBrand}"($|[[:space:]]) ]]; then
    echo "found it !"
fi
Improve this answer
-3

Use [[ instead

if [[ "$PHONE_TYPE" != "NORTEL" ]] || [[ "$PHONE_TYPE" != "NEC" ]] || 
   [[ "$PHONE_TYPE" != "CISCO" ]]
then
echo "Phone type must be nortel,cisco or nec"
exit 1
fi
Improve this answer
1
  • 3
    This is, of course, wrong. [[ vs [ doesn't help with the logic being off. Commented Feb 12, 2019 at 20:10

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.