12

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
}
2
  • 2
    write in your shebang '#!/usr/bin/bash -x' then you'll see what exactly happens.
    – Ziazis
    Apr 24, 2018 at 7:34
  • +1 for telling us it's homework - and for having worked on it hard enough to deserve help.
    – Joe
    Apr 26, 2018 at 5:13

3 Answers 3

20

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
18

@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"
    }
    
4
  • 1
    If you're looking for a terser loop body use sum=$((sum + 1 - element % 2)). Apr 24, 2018 at 8:39
  • 1
    @DavidFoerster Terser and less readable. ;-) Apr 24, 2018 at 10:07
  • @DavidFoerster: I had the same thought that you should just use the mod-2 result directly. (Or better, &1 to check the low bit if that's more readable to you). But we can make it more readable: sum=$((sum + !(element&1) )) to use a boolean inverse instead of +1 - condition. Or just count the odd elements with ((odd += element&1)), and at the end print with echo $(($# - element)), because even = total - odd. Apr 25, 2018 at 10:11
  • Good work, and good to point out little things that beginners miss, e.g. local -i and sum++. May 1, 2018 at 12:06
4

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.

6
  • Interesting approach. Of course this will count 3.0 as an even number; the question wasn’t precise about what form the numbers would take. Apr 25, 2018 at 0:27
  • @G-Man since bash doesn't support floating point arithmetic, it's safe to assume integers
    – muru
    Apr 25, 2018 at 2:02
  • Since the question mentions “even” (and, implicitly, “odd”) numbers, it is somewhat safe to assume that it’s talking about integers.  I don’t see how it’s valid to draw conclusions about the question from the capabilities and limitations of the tool that the user is expected to use.   Remember: the question says this is an assignment — I guess for school, because this is way too trivial to be from a job.  School teachers come up with some crazy things. Apr 25, 2018 at 2:41
  • 2
    Bash can filter a list on its own if we can assume no elements contain whitespace: expand the arg list with odd numbers replaced with the empty string using ${/%/} on the @ array to require a match at the end of the string, inside an array initializer. Print the count. As a one-liner to define and run it: foo(){ evens=( ${@/%*[13579]/} ); echo "${#evens[@]} even numbers"; printf "%s\n" "${evens[@]}"; }; foo 135 212 325 3 6 3 4 5 9 7 2 12310. Includes actually printing the list for debugging. Prints 5 even numbers 212 6 4 2 12310 (on sep. lines) Apr 25, 2018 at 10:24
  • 1
    Probably ZSH has something to properly filter a list, instead of depending on word splitting to rebuild a new array (I was a bit surprised bash didn't; only applying scalar string expansion stuff to every element). For large lists, I wouldn't be surprised if grep was actually faster than bash. Hmm, I wonder if there's a codegolf question where I could post that bash function :P Apr 25, 2018 at 22:30

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .