Take the 2-minute tour ×
Unix & Linux Stack Exchange is a question and answer site for users of Linux, FreeBSD and other Un*x-like operating systems.. It's 100% free, no registration required.
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
:
else
echo "Phone type must be nortel,cisco or nec"
exit
fi

Are there cleaner ways for this type of task?

share|improve this question
add comment

4 Answers

up vote 8 down vote accepted

I guess you're looking for

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

These rules for these equivalents are called De Morgan 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)

wich obviously doesn't work.

share|improve this answer
add comment

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
share|improve this answer
add comment

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
share|improve this answer
add comment

A much shorter way would be:

if [[ $PHONE_TYPE =~ ^(NORTEL|NEC|CISCO)$ ]]; then 
  echo "Phone type must be nortel, cisco or nec."
fi
share|improve this answer
2  
Or without separately anchoring each alternative to the ends of the string: [[ $PHONE_TYPE =~ ^(NORTEL|NEC|CISCO)$ ]]. –  manatwork Mar 14 '13 at 17:01
    
Even better, thanks! I updated the regex. –  0x80 Mar 14 '13 at 17:13
    
hmm i like this short and good looking –  munish Mar 15 '13 at 6:40
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.