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), a1
if an interesting number occurs within the next two miles, or a0
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
or73837
- The digits match one of the values in the
awesome_phrases
array† For incrementing sequences,
0
should come after9
, and not before1
, as in7890
.
‡ For decrementing sequences,0
should come after1
, and not before9
, as in3210
.
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
1 Answer 1
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
)
Explore related questions
See similar questions with these tags.