Home / ubuntu / Questions / 1027693
Accepted
Jedidja
Asked: 2018-04-24 23:19:00 +0800 CST

Mistake in shell function to count even numbers

  • 772

For an assignment I have to write a function that prints the number of even numbers when provided with a sequence of numbers.

I used the piece of code I used for a previous assignment (to print 1 when a number was even and 0 when the number was odd)

My problem now is that my function keeps printing 0. What am I doing wrong?

Here's my script:

#!/usr/bin/bash
# File: nevens.sh

# Write a function called nevens which prints the number of even numbers when provided with a sequence of numbers.
# Check: input nevens 42 6 7 9 33 = output 2

function nevens {

        local sum=0

        for element in $@
        do
                let evencheck=$(( $# % 2 ))
                if [[ $evencheck -eq 0 ]]
                then
                        let sum=$sum+1
                fi
        done

        echo $sum
}
scripts bash functions

3 Answers

  1. You just forgot to replace $# with ($)element in the for loop:

    function nevens {
  local sum=0
  for element in $@; do
    let evencheck=$(( element % 2 ))
    if [[ $evencheck -eq 0 ]]; then
      let sum=sum+1
    fi
  done
  echo $sum
}

    Now to test the function:

    $ nevens 42 6 7 9 33
2
$ nevens 42 6 7 9 33 22
3
$ nevens {1..10..2} # 1 to 10 step 2 → odd numbers only
0
$ nevens {2..10..2} # 2 to 10 step 2 → five even numbers
5
    • 20
  2. Best Answer

    @dessert has found the core problem, I'll give some code review:

    1. The shebang: There is no /usr/bin/bash in Ubuntu. It's /bin/bash.

    2. It's good that you have declared sum local, and avoided polluting the variable namespace outside the function. Additionally, you can declare it an integer variable using the -i option:

      local -i sum=0

    3. Always quote your variables (and parameters)! It's not necessary in this script, but a very good habit to get into:

      for element in "$@"
do

      That said, you can omit the in "$@" here:

      for element
do

      When in <something> is not given, the for loop implicitly loops over the arguments. This can avoid mistakes like forgetting the quotes.

    4. There's no need to calculate and then check the result. You can directly do the calculation in the if:

      if (( (element % 2) == 0 ))
then
    ((sum = sum + 1))
fi

      (( ... )) is the arithmetic context. It's more useful than [[ ... ]] for performing arithmetic checks, and additionally you can omit the $ before variables (which makes it easier to read, IMHO).

    5. If you moved the even-checking part into a separate function, it might improve readability and reusability:

      function evencheck
{
    return $(( $1 % 2 ))
}
function nevens
{
    local -i sum=0
    for element
    do
        # `if` implicitly checks that the returned value/exit status is 0
        if evencheck "$element"
        then
            (( sum++ ))
        fi
    done
    echo "$sum"
}
    • 18

  3. I'm not sure if you're open to other solutions. Also I don't know if you can use external utilities, or if you're purely limited to bash builtins. If you can use grep, for example, your function could be a whole lot simpler:

    function nevens {
    printf "%s\n" "$@" | grep -c '[02468]$'
}

    This puts each input integer on its own line, and then uses grep to count the lines that end in an even digit.

    Update - @PeterCordes pointed out that we can even do this without grep - just pure bash, so long as the input list contains just well formed integers (with no decimal points):

    function nevens{
    evens=( ${@/%*[13579]/} )
    echo "${#evens[@]}"
}

    This works by creating a list called evens by filtering out all the odds, then returning the length of that list.

    • 4