CODEWITHSUNDEEP
bash
DrJessop
DrJessop

Reputation: 472

Passing Scripts as Arguments Bash

This script is incomplete since I will want to do error testing later, but the idea is that ARG is a script and ARG2 is a directory, and ARG should mark all of the files in ARG2. How would I do this in such a way that bash knows the first argument has to be a script and argument 2 is a directory?

ARG=$1
ARG2=$2
CHECK=0
aCount=0
bCount=0
cCount=0
dCount=0
fCount=0

if [ $CHECK -e 0 ]; then
    for files in $ARG2; do
        if [ sh $1 $2 -eq A]; then
            aCount=$((aCount+1))
        elif [ sh $1 $2 -eq B]; 
            bCount=$((bCount+1))
        elif [ sh $1 $2 -eq C]; 
            cCount=$((cCount+1))
        elif [ sh $1 $2 -eq D ];
            dCount=$((dCount+1))
        else;
            fCount=$((fCount+1))
        fi
    done
fi

echo A: $aCount
echo B: $bCount       
echo C: $cCount          
echo D: $dCount
echo F: $fCount

Upvotes: 0

Views: 45

Answers (2)

Paul Hodges
Paul Hodges

Reputation: 15293

@John Kugelman did a great job above. For an alternate take -

declare -A count                   # count is an array
for file in "$dir"/*               # skipping assignments, and $check
do grade=$("$script" "$file")      # grab the output as $grade
   case $grade in                  # look up its value
   [A-D]) (( count[$grade]++ ));;  # use as-is for a-d
       *) (( count['F']++    ));;  # throw everything else in f
   esac
done

for g in A B C D F                 # then for a-f (known values)
do echo "$g: "${count[$g]}         # pull the counts
done

Upvotes: 2

John Kugelman
John Kugelman

Reputation: 361635

There are a variety of errors you could catch by running your script through shellcheck.net.

Corrections:

  • To loop over the files in a directory, write for file in dir/* not for file in dir. The latter just loops once with $file set to the string "dir", rather than iterating over the contents of the directory dir/.

  • [ sh $1 $2 -eq A] is a jumble of shell constructs. You want to capture the output of the script, so you need $(...). You're doing a string check, so you should use == not -eq. Correcting both yields:

    [ $(sh $1 $2) == A ]

  • I'm guessing $2 should be $files, though. The loop variable, yes?

    [ $(sh $1 $files) == A ]

  • There are other miscellaneous mistakes, such as missing thens and not always having a space before ].

Improvements:

  • You should quote everything properly to prevent inadvertent word splitting and glob expansion. 

    [ "$(sh "$1" "$files")" == A ]

  • Let's replace $1 with $script and $files with singular $file.

    [ "$(sh "$script" "$file")" == A ]

  • If the script has a proper shebang line like #!/bin/bash at the top then there's no need to explicitly invoke sh. 

    [ "$("$script" "$file")" == A ]

  • That's all great. Now you have something like this:

    if [ "$("$script" "$file")" == A ]; then
    aCount=$((aCount+1))
elif [ "$("$script" "$file")" == B ]; then
    bCount=$((bCount+1))
elif [ "$("$script" "$file")" == C ]; then
    cCount=$((cCount+1))
elif [ "$("$script" "$file")" == D ]; then
    dCount=$((dCount+1))
else
    fCount=$((fCount+1))
fi

    Awfully repetitious, no? Let's try a case statement instead.

    case "$("$script" "$file")" in
    A) aCount=$((aCount+1));;
    B) bCount=$((bCount+1));;
    C) cCount=$((cCount+1));;
    D) dCount=$((dCount+1));;
    *) fCount=$((fCount+1));;
esac

  • That case statement is still quite complex. Let's break it up to make it easier to parse.

    grade=$("$script" "$file")

case $grade in
    ...
esac

  • Variable names ought to be lowercase. UPPERCASE names are reserved for the shell, so best not to use those. Change COUNT to count.

  • Let's rename ARG and ARG2 to script and dir, respectively. Meaningful names make everything easier to read.

  • var=$((var+1)) can be simplified to ((var += 1)) or ((var++)).

End result:

script=$1
dir=$2

check=0
aCount=0
bCount=0
cCount=0
dCount=0
fCount=0

if ((check == 0)); then
    for file in "$dir"/*; do
        grade=$("$script" "$file")

        case $grade in
            A) ((aCount++));;
            B) ((bCount++));;
            C) ((cCount++));;
            D) ((dCount++));;
            *) ((fCount++));;
        esac
    done
fi

echo "A: $aCount"
echo "B: $bCount"
echo "C: $cCount"
echo "D: $dCount"
echo "F: $fCount"

Upvotes: 4

Related Questions