CODEWITHSUNDEEP
linuxbashif-statement
theuniverseisflat
theuniverseisflat

Reputation: 881

If statement operation not working?

This function is not working. It seems that I'm using too many operations. It works up to second II or operation and then it stops working on the third and 4th. So the line I commented out is not working but the other line is working

Not sure what I'm doing wrong? I do not want to use case statement; maybe there is a better way to do this? I just need to call this function and ensure user entry is correct. It should not be blank and it should be either of the three other entries

get_selection ()
{
count1=1
while [ $count1 = 1 ];
do
echo "enter user or service or item user|service|item :"
read entry
echo entry is $entry
#   if [ -z "$entry" ] || [ $entry != "user" ] || [ $entry != "service" ] || [ $entry!= "item" ]; then
if [ -z "$entry" ] || [ $entry != "user" ] || [  $entry != "service" ]; then
echo "blank or incorrrect entry"
else
echo "entry is correct "
count1=$(( $count1 + 1 ))
fi
done
}

Upvotes: 1

Views: 126

Answers (3)

mklement0
mklement0

Reputation: 440586

To complement @chepner and @naab's helpful answers:

Taking full advantage of bash's [[ ... ]] conditionals (vs. [ ... ]), your original if statement could be written as:

if [[ ! ($entry == "user" || $entry == "service") ]]; then
  echo "blank or incorrrect entry"
else
  echo "entry is correct "
fi

Note that there's no need to test if $entry is blank (with -z "$entry"), since you're only interested in whether the entry matches one of your keywords (and your error message covers both the invalid-input and empty-input cases).

Upvotes: 1

naab
naab

Reputation: 1140

If you don't want to use a case, you could use :

get_selection ()
{
   count1=1
   while [ $count1 = 1 ]; do
      echo "enter user or service or item user|service|item :"
      read entry
      echo entry is $entry

      if [[ -z "$entry" ]] || [[ ! $entry =~ (user|service|item) ]]; then
         echo "blank or incorrrect entry"
      else
         echo "entry is correct "
         count1=$(( $count1 + 1 ))
      fi
   done
}

But I still would recommend the case : 

get_selection ()
{
   count1=1
   while [ $count1 = 1 ]; do
      echo "enter user or service or item user|service|item :"
      read entry
      echo entry is $entry

      case $entry in
         user|service|item)
               echo "entry is correct "
               count1=$(( $count1 + 1 ))
               break
               ;;
         *)
               echo "blank or incorrrect entry"
      esac

   done
}

Which is clear about what you are checking.

Upvotes: 3

chepner
chepner

Reputation: 532448

No matter what the value of entry is, it will fail to match at least one of "user" or "service" (i.e., it cannot equal both of them). You want

if [ -z "$entry" ] || { [ "$entry" != "user" ] && [ "$entry" != "service" ]; }; then

You can use a case statement and a different looping condition to simplify the function:

get_selection () {
    while true; do
       echo "Enter user or service or item: "
       read entry
       case $entry in
           "") echo "blank entry" ;;
           user|service|item) echo "entry is correct"; break; ;;
           * ) echo "incorrect entry" ;;
       esac
    done
}

However, you are essentially recreating the select command:

select entry in user service item; do
done

Upvotes: 5

Related Questions