1
\$\begingroup\$

Question 7 from Lab 3 of Berkeley's CS61A course says:

The following examples of recursive functions show some examples of common recursion mistakes. Fix them so that they work as intended. [...]

def even_digits(n):
    """Return the percentage of digits in n that are even.

    >>> even_digits(23479837) # 3 / 8
    0.375
    """
    if n == 0:
        return num_digits / num_evens
    num_digits, num_evens = 0, 0
    if n % 2 == 0:
        counter += 1
    num_evens += 1
    return (even_digits(n // 10) + num_evens) / counter

As per the assignment, below code is suppose to solve this problem, recursively (MUST not be non-recursive).

def even_digits(n, count_even = 0, count = 0):
    """Return the percentage of digits in n that are even.

    >>> even_digits(23479837) # 3 / 8
    0.375
    """
    if n == 0:
        return count_even / count
    if (n % 2) == 0:
        return even_digits(n // 10, count_even + 1, count + 1)
    else:
        return even_digits(n // 10, count_even, count + 1)

Is this code readable? Can we improve this solution?

\$\endgroup\$
8
  • \$\begingroup\$ Why does it have to be a recursive solution? Solving this non-recursively looks much more logical. \$\endgroup\$
    – Lars Ebert
    Jun 30, 2015 at 9:29
  • 1
    \$\begingroup\$ Because the assignment says so. \$\endgroup\$ Jun 30, 2015 at 9:31
  • \$\begingroup\$ @skull are you new to codereview? \$\endgroup\$ Jun 30, 2015 at 9:59
  • \$\begingroup\$ Can you quote the assignment (or link to it), please? Also, the example seems wrong: the correct percentage is 37.5%, not 0.375%. \$\endgroup\$ Jun 30, 2015 at 11:21
  • \$\begingroup\$ @GarethRees this is the link \$\endgroup\$ Jun 30, 2015 at 12:15

4 Answers 4

6
\$\begingroup\$

There is a serious error in your code. If the function is called as even_digits(0), then it causes the following error:

ZeroDivisionError: integer division or modulo by zero

since in the first if you divide by count which is 0.

So, before improving the code, you should check if it is correct.

ADDED:

THE CASE OF n = 0

It is interesting to understand what should be the result in case the input (that is n) is 0.

The problem is labeled as:

Percentage of digits in n that are even

and the natural interpretation is:

Percentage of digits in the decimal representation of n that are even

So, what result should give the function when provided with the value 0? I argue it should be 1, since every decimal representation of 0 has only zeroes as digits, so it has a 100% even digits.

\$\endgroup\$
7
  • \$\begingroup\$ Sorry, Is this the right solution? \$\endgroup\$ Jun 30, 2015 at 9:37
  • 1
    \$\begingroup\$ @Renzo I'm not convinced this is a bug, since the fraction of even digits in the number is undefined. \$\endgroup\$
    – Veedrac
    Jun 30, 2015 at 9:38
  • \$\begingroup\$ @overexchange Don't use assert for input validation, since it is removed when the -O flag is used. \$\endgroup\$
    – Veedrac
    Jun 30, 2015 at 9:39
  • \$\begingroup\$ @Veedrac Do I need to use try-catch for input validation? \$\endgroup\$ Jun 30, 2015 at 9:48
  • \$\begingroup\$ You could apply the same solution as @TheBlackCat \$\endgroup\$
    – Renzo
    Jun 30, 2015 at 10:45
4
\$\begingroup\$

Overall it is good. I would probably clarify that n is an integer (this won't work with floats) in the docstring. I would probably either document count_even and count or change them to _count_even and _count. I would also comment the three cases. I was able to figure it out, but it took me a few moments.

Also, 0 is False in python, so you don't need to explicitly test for == 0.

However, I think this would be better as a loop.

def even_digits(n, count_even = 0, count = 0):
    count = 0
    count_even = 0
    while n:
       count += 1
       if not n%2:
           count_even += 1
       n //= 10
    return count_even/count if count else 1

Edit: I change the return line (last line) to handle the division-by-zero case

\$\endgroup\$
6
  • \$\begingroup\$ Always love the tail recursion eliminated. \$\endgroup\$
    – vnp
    Jun 30, 2015 at 7:53
  • 1
    \$\begingroup\$ @TheBlackCat, the same error that I described in my answer is present in your answer (division by 0). \$\endgroup\$
    – Renzo
    Jun 30, 2015 at 8:47
  • \$\begingroup\$ @Renzo I fixed it \$\endgroup\$ Jun 30, 2015 at 10:11
  • 2
    \$\begingroup\$ @TheBlackCat Shouldn't it return 1 is case of 0 input? After all, the only digit (in normal representation) of number 0 is even. \$\endgroup\$
    – Cthulhu
    Jun 30, 2015 at 11:22
  • \$\begingroup\$ @Cthulhu Good point, fixed again. \$\endgroup\$ Jun 30, 2015 at 12:00
2
\$\begingroup\$

even_digits() doesn't feel right as the function name. I would not expect a percentage being returned from that function. Maybe a count? Perhaps a collection of the individual even digits?

Don't use else after return.

    if (n % 2) == 0:
    return even_digits(n // 10, count_even + 1, count + 1)
else:
    return even_digits(n // 10, count_even, count + 1)
if (n % 2) == 0:
    return even_digits(n // 10, count_even + 1, count + 1)

return even_digits(n // 10, count_even, count + 1)
\$\endgroup\$
1
\$\begingroup\$

You could use a count method:

def percent_even(number):
    try:
        number = int(number)  # Check the number is an integer
    except ValueError:
        return None           # Return None if number is not a valid integer
    number = str(number)      # Make the number a string (for count)
    if number:
        evens = [str(even) for even in list(range(0,9,2))]       # Generate a list of even digits as strings
        count = sum([number.count(digit) for digit in evens])    # Count the number of each digit
        percentage = (float(count) / len(number)) * 100          # Calculate a percentage
        return percentage
    else:
        return None        # Return None if the string was empty
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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