4
\$\begingroup\$

I'm currently a 3rd year computer science student and lecturers have never checked the readability and maintainability of code. We were only marked on our outputs and as such I've written some truly horrible looking code in the past. I'm trying to work on that and one thing I've learned is that it's a good idea to put things in functions to make testing and maintaining code easier.

I completed the following question:

Write the function that parses the mileage number input, and returns a 2 if the number is "interesting" (see below), a 1 if an interesting number occurs within the next two miles, or a 0 if the number is not interesting.

"Interesting" Numbers

Interesting numbers are 3-or-more digit numbers that meet one or more of the following criteria:

  • Any digit followed by all zeros: 100, 90000
  • Every digit is the same number: 1111
  • The digits are sequential, incementing: 1234
  • The digits are sequential, decrementing: 4321
  • The digits are a palindrome: 1221 or 73837
  • The digits match one of the values in the awesome_phrases array

For incrementing sequences, 0 should come after 9, and not before 1, as in 7890.
For decrementing sequences, 0 should come after 1, and not before 9, as in 3210.

My code is:

def number_followed_by_zeroes(number):
    #divides a number until there are no more trailing zeroes
    while(number%10 == 0):
        number = number/10
    if(number>10):    
        return False
    return True

def repeating_digit(number):
    #stores all digits in a set(duplicates not allowed in sets)
    #converts to list of digits then stores in set
    unique_digits = set( list( str( number ) ) )
    #All digits were the same
    if(len(unique_digits) == 1):
        return True

    return False

def increasing_sequential(number):
    #store all digits in a list
    digits = list( str( number ) )

    #Sorted function doesn't alter original list
    sorted_list = sorted(digits)



    #While loop ensures all zeros are at the end of sorted list
    #As specified by question
    zero_edge = sorted_list[0]
    while(zero_edge == "0"):
        sorted_list.append(sorted_list[0])
        del sorted_list[0]
        zero_edge = sorted_list[0]

    #Make sure it's incrementally increasing
    #consec_num stands for consecutive_number
    consec_num = int(sorted_list[0])
    for i in sorted_list:

        if consec_num != int(i):
            return False
        consec_num += 1
        if i == "9":
            consec_num = 0

    #if sorted and original list are the same then
    #number was sorted
    if(sorted_list == digits):
        return True
    return False

def decreasing_sequential(numbers):
    #similar to increasing_sequential but in reverse
    #No need to alter normal sorting in regards to zero
    digits = list( str( numbers ) )
    sorted_list = sorted( digits )

    #Make sure it's incrementally increasing
    #consec_num stands for consecutive_number
    consec_num = int(sorted_list[0])
    for i in sorted_list:
        if consec_num != int(i):
            return False
        consec_num += 1

    sorted_list.reverse()

    if(sorted_list == digits):
        return True
    return False

def palindrome(numbers):
    digits = list( str( numbers ) )
    #Splits the digits into half storing each in their own list
    first_half = digits[0:(int(len(digits) /2))]
    second_half = digits[int(len(digits)/2):]

    #Runs if there were an odd number of digits, deletes the middle number
    if(len(first_half) != len(second_half)):
        del second_half[0]

    #Since a second half of string would be reverse copy of first in a palindrome

    second_half.reverse()

    if(second_half == first_half):
        return True
    return False

def is_interesting(number, awesome_phrases):

    if(number<98):
        return 0

    if number in awesome_phrases:
        return 2

    #if the original number is awesome
    if(number > 99 and (number_followed_by_zeroes(number) or repeating_digit(number) or
        increasing_sequential(number) or decreasing_sequential(number) or
        palindrome(number)) ):
        return 2
    #if the next 2 numbers are awesome

    for i in range(1,3):
        number += 1
        if number in awesome_phrases:
            return 1

        if(number_followed_by_zeroes(number) or repeating_digit(number) or
        increasing_sequential(number) or decreasing_sequential(number) or
        palindrome(number) ):
            return 1


    return 0
    pass

Would I be fine if I coded this at a business or have I created too many functions or taken too rudimentary approaches to each function?

One thing I did consider is to add another function to make the number is above 99

def great_enough(number):
    if number>99:
        return True
\$\endgroup\$
5
\$\begingroup\$

Confusion

I think that the challenge is poorly posed, because of an ambiguity in the term "interesting". On one hand, it says "A number is only interesting if it is greater than 99!" On the other hand, the specification calls for the is_interesting() function to return "1 if an interesting number occurs with the next two miles". So, what should is_interesting(98, []) return? By my interpretation, it should return 1, because 98 itself is not interesting, but it is nearly 100, which is 1 followed by two zeroes. Your code does that, but it's not obvious why it behaves that way, especially since you wrote if(number<98): return 0. (Why is 98 special? Because it's 2 less than a three-digit number. So, you've encoded the within-two rule in two places: if(number<98): return 0 and for i in range(1,3) — and it's bad practice to encode a rule twice.)

It's unfortunate that the challenge calls for an is_interesting() function to be written, when it would be more accurately described as "is interesting or approaching interesting". Given the unfortunate specification, though, I think it would be a good idea to write a function is_intrinsically_interesting(), which returns either True or False, without considering the next two integers. That would eliminate the redundancy in your implementation of is_interesting().

Your comments confuse awesomeness (whether a number appears in the awesome_phrases list) with interestingness.

Functions

You and the challenge talk about testing, but you haven't included any tests. I suggest writing doctests.

A good naming convention to follow is is_… for predicates (functions that return either True or False.

Most of your functions take a number parameter, but for some reason you wrote decreasing_sequential(numbers) and palindrome(numbers), which is confusing.

Since most of these tests operate on digits, consider accepting a list of digits instead of the integer.

Expressiveness

Some of your functions are rather long. Each test could be written as a one-liner, or nearly so.

Instead of writing

if condition:
    return True
return False

… you should just write return condition (or, to explicitly coerce the result to a boolean, return bool(condition).

The pass on the very last line is dead code.

The palindrome() function should be written such that you don't need separate even- and odd-length cases.

Suggested solution

def is_digit_followed_by_all_zeroes(digits):
    """
    >>> is_digit_followed_by_all_zeroes([1, 0, 0])
    True
    >>> is_digit_followed_by_all_zeroes([9, 0, 0, 0, 0])
    True
    >>> is_digit_followed_by_all_zeroes([1, 0, 1])
    False
    """
    return all(digit == 0 for digit in digits[1:])

def is_repeated_digit(digits):
    """
    >>> is_repeated_digit([1, 1, 1, 1])
    True
    >>> is_repeated_digit([2, 1, 2])
    False
    """
    return 1 == len(set(digits))

def is_increasing_sequential(digits):
    """
    >>> is_increasing_sequential([1, 2, 3, 4])
    True
    >>> is_increasing_sequential([7, 8, 9, 0])
    True
    >>> is_increasing_sequential([9, 0, 1])
    False
    """
    ORDER = [1, 2, 3, 4, 5, 6, 7, 8, 9, 0]
    return all(
        ORDER.index(i) + 1 == ORDER.index(j)
        for i, j in zip(digits, digits[1:])
    )

def is_decreasing_sequential(digits):
    """
    >>> is_decreasing_sequential([4, 3, 2, 1])
    True
    >>> is_decreasing_sequential([3, 2, 1, 0])
    True
    >>> is_decreasing_sequential([2, 1, 0, 9])
    False
    """
    return all(
        i - 1 == j
        for i, j in zip(digits, digits[1:])
    )

def is_palindrome(digits):
    """
    >>> is_palindrome([7,3,8,3,7])
    True
    >>> is_palindrome([1,2,2,1])
    True
    >>> is_palindrome([1,2,3,1])
    False
    """
    half_len = (len(digits) + 1) // 2   # Half the length, rounding up
    return digits[:half_len] == digits[-1 : -half_len-1 : -1]

def is_intrinsically_interesting(number, awesome_phrases):
    """
    Test whether the number itself is "interesting".
    """
    digits = [int(d) for d in str(number)]
    return number > 99 and (
        is_digit_followed_by_all_zeroes(digits) or
        is_repeated_digit(digits) or
        is_increasing_sequential(digits) or
        is_decreasing_sequential(digits) or
        is_palindrome(digits)
    ) or number in awesome_phrases

def is_interesting(number, awesome_phrases):
    return (
        2 if is_intrinsically_interesting(number, awesome_phrases) else
        1 if is_intrinsically_interesting(number + 1, awesome_phrases) else
        1 if is_intrinsically_interesting(number + 2, awesome_phrases) else
        0
    )
\$\endgroup\$

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.