TDD Hackerrank: Library Fines

Question

From this hackerrank:

Problem Statement

The Head Librarian at a library wants you to make a program that calculates the fine for returning the book after the return date. You are given the actual and the expected return dates. Calculate the fine as follows:

• If the book is returned on or before the expected return date, no fine will be charged, in other words fine is 0.
• If the book is returned in the same calendar month as the expected return date, Fine = 15 Hackos × Number of late days
• If the book is not returned in the same calendar month but in the same calendar year as the expected return date, Fine = 500 Hackos × Number of late months
• If the book is not returned in the same calendar year, the fine is fixed at 10000 Hackos. Input

You are given the actual and the expected return dates in D M Y format in two separate lines. The first line contains the D M Y values for the actual return date and the next line contains the D M Y values for the expected return date.

My development philosophy was to TDD this. First write code that reads the date correctly, then start implementing the test cases from the problem statement.

I did not put anything in a separate class.

My code passed all their test cases but looks ugly as heck. Any suggestions for improvement would be appreciated!

import datetime

def testFine(dueDate, returnDate, actualFine):
    fine = getFine(dueDate, returnDate)
    if (fine != actualFine):
        print( "Test FAILED for duedate %s and returnDate %s with fine: %d but expected:%d." %( dueDate, returnDate, fine, actualFine) )


    else:
       print( "Test passed for duedate %s and returnDate %s with fine: %d." % ( dueDate, returnDate, fine) )

def tests():

    #If the book is not returned in the same calendar year, the fine is fixed at 10000 Hackos.
    testFine(datetime.date(2015,5,5), datetime.date(2016,5,6),10000)

    #If the book is returned on or before the expected return date, no fine will be charged, in other words fine is 0.
    testFine(datetime.date(2015,5,5), datetime.date(2015,5,5),0)
    testFine(datetime.date(2015,5,5), datetime.date(2015,5,3),0)
    #If the book is returned in the same calendar month as the expected return date, Fine = 15 Hackos × Number of late days

    testFine(datetime.date(2015,5,5), datetime.date(2015,5,6),15)
    testFine(datetime.date(2015,2,5), datetime.date(2015,2,10),75)

    #If the book is not returned in the same calendar month but in the same calendar year as the expected return date, Fine = 500 Hackos × Number of late months

    testFine(datetime.date(2015,2,5), datetime.date(2015,3,1),500)
    testFine(datetime.date(2015,2,5), datetime.date(2015,6,1),2000)


def getFine(dueDate, returnDate):

    if (returnDate.year > dueDate.year):
        return 10000

    if (returnDate.year == dueDate.year and returnDate.month == dueDate.month and returnDate.day > dueDate.day):
        return 15*(returnDate.day-dueDate.day)

    if (returnDate.year == dueDate.year and returnDate.month > dueDate.month):
        return 500*(returnDate.month-dueDate.month)

    return 0

def getDateFromCin():
    s = input()
    nums = [int(n) for n in s.split(" ")]
    return datetime.date(nums[2], nums[1],nums[0])


def testFunc():
    testline = input()

    print (testline + "test")
    return

def cinMain():
    returnDate = getDateFromCin()
    dueDate = getDateFromCin()
    fine = getFine(dueDate, returnDate)
    print (fine)

def main():
    tests()

cinMain()

Answer 1

Avoid so many temporary variables

In Python, temporary variables are used only if they benefit readability, not if they hurt it:

def getDateFromCin():
    s = input()
    nums = [int(n) for n in s.split(" ")]
    return datetime.date(nums[2], nums[1],nums[0])

Becomes:

def getDateFromCin():
    nums = [int(n) for n in input().split(" ")]
    return datetime.date(nums[2], nums[1], nums[0])

Actually, @QPaysTaxes suggested to use the splat (*) operator, and I also think it is a good idea, this shortens to function further to just:

return datetime.date(* [int(n) for n in reverse(input().split(" "))])

And:

def testFunc():
    testline = input()

    print (testline + "test")
    return

Becomes:

def testFunc():
    print (input() + "test")
    # return is not needed

And:

def cinMain():
    returnDate = getDateFromCin()
    dueDate = getDateFromCin()
    fine = getFine(dueDate, returnDate)
    print (fine)

Becomes:

def cinMain():
    returnDate = getDateFromCin()
    dueDate = getDateFromCin()
    print( getFine(dueDate, returnDate) )

Small simplifications like this are nothing interesting on their own but when you make a lot of them they start to add up making the code cleaner.

Simplify your booleans

if (returnDate.year == dueDate.year and returnDate.month == dueDate.month and returnDate.day > dueDate.day):
    return 15*(returnDate.day-dueDate.day)

If you got here, then returnDate.year == dueDate.year is surely True, as True and x == x you can simplify:

if (returnDate.month == dueDate.month and returnDate.day > dueDate.day):
        return 15*(returnDate.day-dueDate.day)

Remember to add:

if returnDate <= dueDate:
    return 0

At the start of the function to preserve correctness.

Use a testing framework

You made some work to run the automated tests, but there are already tools for that, I suggest doctest, it is very easy to use and doubles as documentation:

def book_return_fine(dueDate, returnDate):
    """
    >>> book_return_fine(datetime.date(2015,5,5), datetime.date(2016,5,6))
    10000
    """
    if (returnDate.year > dueDate.year):
        return 10000

    if (returnDate.month == dueDate.month and returnDate.day > dueDate.day):
        return 15*(returnDate.day-dueDate.day)

    if (returnDate.month > dueDate.month):
        return 500*(returnDate.month-dueDate.month)

    return 0

Just add import doctest at the start and doctest.testmod() at the end.

Naming

Naming is hard, I really do not like your names.

• get is a specific OO concept and gives almost no info about what the function does.

• cin is a C++ term, not everyone may remember what it stands for.

• nums well, if you convert them to int they are nums but this really does not give me much info...

• snake_case is the common Python naming style, not camelCase.

Spacing

Asinnaturallanguage,spacingthingsouthelpsthereaderreadfasterandeasier.

As in natural language, spacing things out helps the reader read faster and easier.

For example:

return 15*(returnDate.day-dueDate.day)

Becomes:

return 15 * (returnDate.day - dueDate.day)

Answer 2

You're using the old % formatting, the new style you should use is str.format.

if (fine != actualFine):
    print("Test FAILED for duedate {} and returnDate {} with fine: {} but expected:{}.".format(dueDate, returnDate, fine, actualFine))

You don't need to care what type your data is, as long as it can be turned into a string using str or repr then Python can interpret it to insert in your string here in place of the {}s.

You can leave out the brackets around your if tests, if returnDate.year > dueDate.year: works fine and is more Pythonic. You'd only really need brackets to separate out the order of operations correctly, eg if (A and B) or C:

Consider setting up 10000, 15 and 500 as constants. YEARLY_FINE, MONTHLY_FINE and DAILY_FINE are much more readable values to calculate with.