Ggthjy

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

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

    )