0

I am using if statement with multiple condition in bash.

How can I reduce the following line syntax. So that it looks good from design point of you.

 if [ "$1" != "-l" ] && [ "$1" != "-a" ] && [ "$1" != "-h" ] && [ "$1" != "" ] && [ "$1" =  "-d" ] || [ "$1" = "-mv" ] || [ "$1" = "-dv" ] || [ "$1" = "-mr" ] || [ "$1" = "-dr" ];

Thanks

Singh
  • 41
  • 7

3 Answers3

1

Use pattern matching.

if [[ $1 && $1 != -[lah] && $1 == -@(d|mv|dv|mr|dr) ]]; then

@(...) is an example of an extended pattern, which should be recognized by default inside [[ ... ]] in recent versions of bash. If you version is not so recent, add shopt -s extglob to the beginning of your script.

In fact, you can drop the $1 && $1 != -[lah] because its truth would be implied by the truth of $1 == -@(...).

if [[ $1 == -@(d|mv|dv|mr|dr) ]]; then

You could also just use a POSIX-compliant case statement:

case $1 of
 -d|-mv|-dv|-mr|-dr) echo good option ;;
 *) echo bad option ;;
esac
chepner
  • 497,756
  • 71
  • 530
  • 681
0

You can create 2 arrays for matching and non matching values and check if element $1 matches any element in the array or not like below. 

nonmatch_array=( "-l" "-a" "-h" "" )
match_array=(  "-d" "-mv" "-dv" "-mr" "-dr" )

if [ `echo ${match_array[@]} | grep "$1"` ] || ! [ `echo ${nonmatch_array[@]} | grep "$1"` ] ; then 
    echo "is in array"
else 
    echo "is not in array"
fi

Hope it should work for you.

Nishu Tayal
  • 20,106
  • 8
  • 49
  • 101
  • `grep -q` always emits nothing, and ```[ `...` ]``` is evaluating the stdout of your `...` as a test (if it's a single word without meaning to the test command, checking whether that word is or is not empty; but with `grep -q`, it's **always** empty). – Charles Duffy Oct 14 '16 at 21:37
  • @CharlesDuffy: My bad, somehow missed that.. updated the answer. Thanks for noticing. – Nishu Tayal Oct 14 '16 at 21:42
  • There's still improvement to be made here -- using `grep` is very slow; consider rewriting to use an associative array to do all the tests in-process in O(1) time. – Charles Duffy Oct 14 '16 at 21:42
  • ...or, even if you don't do that, `[[ $string = $target ]]` will tell you whether `$string` matches `$target` much faster than calling out to `grep` will. (Also, `grep` is looking for a substring, but if the OP wants nonmatch_array to be empty, *everything* is a substring of `""`, so that's obviously an incorrect interpretation). – Charles Duffy Oct 14 '16 at 21:44
  • Agreed for pattern match. But in the case of substring match, it can match other strings as well, which is not needed. As per the OP's question, he wants to match exact. – Nishu Tayal Oct 14 '16 at 21:47
  • `grep` is not an exact match without arguments you aren't giving it. Thus, I was explaining a substring match (pre-editing) *because that's what your code already did*. – Charles Duffy Oct 14 '16 at 21:47
  • `[[ $string = "$target" ]]` is already implicitly anchored, and is thus not a substring match; it would be `[[ $string = *"$target"* ]]` to be a substring match. – Charles Duffy Oct 14 '16 at 21:49
0

First try to limit the length of the code on 1 line.

if [  [ "$1" != "-l" ] 
   && [ "$1" != "-a" ]
   && [ "$1" != "-h" ] 
   && [ -n "$1" ] 
   && ( [ "$1" = "-d" ]
   || [ "$1" = "-mv" ]
   || [ "$1" = "-dv" ]
   || [ "$1" = "-mr" ]
   || [ "$1" = "-dr" ] ) ];

I added braces, to make clear what you mean with the or's.

Now you can combine all matches with a regular expression:

   if [[ ! ("$a" =~ ^-(l|a|h|d|)$)
      &&    "$a" =~ ^-(mv|dv|mr|dr)$ ]]; then
      echo "Yes $a matches"
   fi

but reconsider what you are testing. The test will only be true when it matches -mv/-dv/-mr/-dr, so you do not need to test for the options lah.

   if [[ "$a" =~ ^-(d|mv|dv|mr|dr)$ ]]; then
      echo "Yes $a matches"
   fi

You can use a variable for extracting the options:

   options="d|mv|dv|mr|dr"
   if [[ "$a" =~ ^-(${options})$ ]]; then
      echo "Yes $a matches"
   fi

Everytime the code is becoming hard to read (and also for long code or repeating statements), you should consider using a function.
The next function is short, but hard to read:

options="d|mv|dv|mr|dr"
function checkoption1 {
   [[ "$a" =~ ^-(${options})$ ]]
}
checkoption1 "$a" &&
   echo "Yes $a matches"

I would choose for a slightly more verbose function. I will include your original tests for lah for showing the possibilities.

# checkoption return 0 for match,
# returns 1 for forbidden option
# returns 2 for undefined option
function checkoption2 {
   case "$1" in
      -d|-mv|-dv|-mr|-dr) return 0 ;;
      -l|-a|-h|"")  return 1;;
      *) return 2;;
   esac
}
checkoption2 "$a" &&
   echo "Yes $a matches"

You should make some testruns before accepting your code.
I have made some tests with a small loop (now all answers together)

function checkoption1 {
   [[ "$a" =~ ^-(${options})$ ]]
}

# checkoption return 0 for match,
# returns 1 for forbidden option
# returns 2 for undefined option
function checkoption2 {
   case "$1" in
      -d|-mv|-dv|-mr|-dr) return 0 ;;
      -l|-a|-h|"")  return 1;;
      *) return 2;;
   esac
}

for a in -mv mv -mvx -ms -mr -dr; do
       if [[ ! ("$a" =~ ^-(l|a|h|)$)
          &&    "$a" =~ ^-(d|mv|dv|mr|dr)$ ]]; then
          echo "Yes $a matches"
       fi
       if [[ "$a" =~ ^-(d|mv|dv|mr|dr)$ ]]; then
          echo "Yes $a matches"
       fi
       options="d|mv|dv|mr|dr"
       if [[ "$a" =~ ^-(${options})$ ]]; then
          echo "Yes $a matches"
       fi
       checkoption1 "$a" &&
          echo "Yes $a matches"
       checkoption2 "$a" &&
          echo "Yes $a matches 2"
 done
Walter A
  • 19,067
  • 2
  • 23
  • 43
  • Some of this I'm quite sure is nonportable, if correct at all -- such as nested `[`s, or native logic operators (such as `&&`) within that nested context. And using `( ... )` instead of `{ ...; }` for grouping is taking a nontrivial efficiency hit. – Charles Duffy Oct 16 '16 at 01:23