# Simple greeting program based with timezone offset

I made a simple set of functions that returns a greeting based on the current time, the user's timezone offset, and the user's level. For context I wrote this as a part of a larger chat bot application. I am not sure if there is a better way to handle the if/elif/else blocks in both functions.

Please let me know of places of improvement.

"""A module for creating greeting strings based on a user's timezone
offset and user's level."""

import time

def get_part_of_day(user_tz_offset, time_now):
"""Return part of day depending on time_now and the user's timzone
offset value.

user_tz_offset - integer of user's time zone offset in hours
time_now - UTC time in seconds

From  -  To  => part of day
---------------------------
00:00 - 04:59 => midnight
05:00 - 06:59 => dawn
07:00 - 10:59 => morning
11:00 - 12:59 => noon
13:00 - 16:59 => afternoon
17:00 - 18:59 => dusk
19:00 - 20:59 => evening
21:00 - 23:59 => night
"""
user_time = time_now + (user_tz_offset*60*60)
# gmtime[3] is tm_hour
user_hour = time.gmtime(user_time)[3]

if 0 <= user_hour < 5:
return 'midnight'
elif 5 <= user_hour < 7:
return 'dawn'
elif 7 <= user_hour < 11:
return 'morning'
elif 11 <= user_hour < 13:
return 'noon'
elif 13 <= user_hour < 17:
return 'afternoon'
elif 17 <= user_hour < 19:
return 'dusk'
elif 19 <= user_hour < 21:
return 'evening'
else:
return 'night'

"""Return greeting string based on user's level and part of day.

level - integer of user's level
part_of_day - string from function get_part_of_day
"""

greetings = {
'dawn': 'Good early morning',
'morning': 'Good morning',
'afternoon': 'Good afternoon',
'dusk': 'Good afternoon',
'evening': 'Good evening',
}

# Use generic 'Hi' when specific greeting is not implemented
greeting = greetings.get(part_of_day, 'Hi')

if level == 0:
comma = ','
full_stop = '.'
elif level == 1:
comma = ','
full_stop = '!'
else:
comma = ''
full_stop = '!!'

return '%s%s %s%s' % (greeting, comma, username, full_stop)


I also wrote tests. This is my first unittest that I wrote. I'm not sure if I'm covering too little, too much, missed corner cases, etc. Am I doing these correctly?

import unittest
from greetings import get_part_of_day, choose_greeting

class TestGreeting(unittest.TestCase):

def setUp(self):
pass

def test_part_of_day_0offset_local_time0(self):
# 2015-04-01 0:00:00 midnight
test_time = 1427846400
user_tz_offset = 0
part_of_day = get_part_of_day(user_tz_offset, test_time)
self.assertEqual(part_of_day, 'midnight')

def test_part_of_day_0offset_local_time13(self):
# 2015-04-01 13:05:20
test_time= 1429967120
user_tz_offset = 0
part_of_day = get_part_of_day(user_tz_offset, test_time)
self.assertEqual(part_of_day, 'afternoon')

def test_part_of_day_minus9offset_local_time5(self):
# 2012-02-29 05:20:59 (-9)
# 2012-02-29 14:20:59 UTC
test_time = 1330525259
user_tz_offset = -9
part_of_day = get_part_of_day(user_tz_offset, test_time)
self.assertEqual(part_of_day, 'dawn')

def test_part_of_day_minus4offset_local_time22(self):
# 2030-03-09 22:00:59 (-4)
# 2030-03-10 02:00:59 UTC
test_time = 1899338459
user_tz_offset = -4
part_of_day = get_part_of_day(user_tz_offset, test_time)
self.assertEqual(part_of_day, 'night')

def test_part_of_day_11offset_local_time5(self):
# 2015-04-25 05:00:00 (+11)
# 2015-04-24 18:00:00 UTC
test_time = 1429898400
user_tz_offset = 11
part_of_day = get_part_of_day(user_tz_offset, test_time)
self.assertEqual(part_of_day, 'dawn')

def test_choose_greeting_testuser_0_dawn(self):
reply = 'Good early morning, TestUser.'

def test_choose_greeting_testuser_0_midnight(self):

def test_choose_greeting_testuser_0_evening(self):

def test_choose_greeting_testuser_1_afternoon(self):

def test_choose_greeting_testuser_2_dusk(self):

def test_choose_greeting_testuser_2_morning(self):

def test_choose_greeting_testuser_0_noon(self):

def test_choose_greeting_testuser_1_noon(self):

def test_choose_greeting_testuser_2_noon(self):
reply = 'Hi Level2User!!' # no comma

if __name__ == '__main__':
unittest.main()


Output:

----------------------------------------------------------------------
Ran 14 tests in 0.037s

OK


### Redundancy in the if-else chain

Your if-else chain will be easier to maintain if you drop the redundant lower bound checks, like this:

if user_hour < 5:
return 'midnight'
elif user_hour < 7:
return 'dawn'
elif user_hour < 11:
return 'morning'
# ... and so on


Btw, another alternative to the if-else chain is to define a table of hour to "part of day" like this:

part_of_day_table = (
['midnight'] * 5 +
['dawn'] * 2 +
['morning'] * 4 +
# ... and so on
)
assert len(part_of_day_table) == 24

def part_of_day(hour):
assert 0 <= hour < 24
return part_of_day_table[hour]


### Writing unit tests

First of all, it's great that you're writing unit tests. Keep it up!

In assert statements, it's customary to put the expected (= fixed) value on the left side, and the actual (= computed) value on the right. So instead of this:

    part_of_day = get_part_of_day(user_tz_offset, test_time)
self.assertEqual(part_of_day, 'dawn')


Prefer this:

    part_of_day = get_part_of_day(user_tz_offset, test_time)
self.assertEqual('dawn', part_of_day)


Another thing, the way you defined the test times is not ergonomic:

    # 2015-04-01 13:05:20
test_time= 1429967120


It's good that you added in a comment the meaning of 1429967120, but if I want to tweak it, or add another test with +1 hour, I'll have to figure out a way to convert between human readable times and seconds. It would better if I could readily edit human readable code in the test itself, like this:

    test_time = time.strptime('2015-04-01 13:05', '%Y-%m-%d %H:%M')


or even just:

    test_time = time.strptime('13:05', '%H:%M')


In this version there's no more need for the comment, and easier to tweak and maintain.

• I will definitely use your suggestion about the test_time. The time_of_day_table tuple is a neat method!Thank you for the review. – d_dd Apr 26 '15 at 17:58

You could use a dictionary for your case conditions:

def midnight():
print "It's midnight!"

def dawn():
print "Tis dawn"

def morning():
print 'good morning'

TOD = {00 : midnight,
01: midnight,
02: midnight,
03: midnight,
04: midnight,
05: dawn,
06: dawn,
07: morning
}

TEST = 05
TOD[TEST]()


It should find the key in the dictionary and perform the method defined. This would also avoid you from having to have multiple returns in your function which may be frowned upon.

I'm not sure if this is more readable than your code, but it may be a little more OO friendly. I fear this is not any better but would be great to get other peer reviews.

• I never thought about using a dictionary with function values. It would change some of the code if I had to return a function but I will play around with this. Thanks for the idea! – d_dd Apr 26 '15 at 18:01

This isn't a complete review, but I've two points for you:

1. Spelling

Not to be nitpicking, but consistency is important. I've seen accounts of timezone, timzone and time zone. They're comments so it will work just fine, but if you're looking for inconsistencies in code you'll also find the inconsistencies in the comments. Inconsistencies are bad, regardless of whether they're code or comments.

2. Unit tests

You asked about whether you have too many or not enough unit tests. A good amount of unit tests cover at least 90% of all possible combinations, if possible ALL combinations. You can't cover 90% of all possible time combinations, so you'll just have to pick a few for that.

Your unit tests could have been more generic:

def test_choose_greeting_testuser_2_noon(self):
reply = 'Hi Level2User!!' # no comma

That's two counts of greeting, two counts of Level2USer (and one testuser_2, why are those inconsistent?) and two counts of noon. That's needless repetition.