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

@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"
    }
    
  • 1
    If you're looking for a terser loop body use sum=$((sum + 1 - element % 2)). – David Foerster Apr 24 '18 at 8:39
  • 1
    @DavidFoerster Terser and less readable. ;-) – Konrad Rudolph Apr 24 '18 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. – Peter Cordes Apr 25 '18 at 10:11
  • Good work, and good to point out little things that beginners miss, e.g. local -i and sum++. – Paddy Landau May 1 '18 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.

  • 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. – G-Man Apr 25 '18 at 0:27
  • @G-Man since bash doesn't support floating point arithmetic, it's safe to assume integers – muru Apr 25 '18 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. – G-Man Apr 25 '18 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) – Peter Cordes Apr 25 '18 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 – Peter Cordes Apr 25 '18 at 22:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.