Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Function to use several blocks of if:elif:else: that all take slightly different conditional statements. I'm trying to format integers into a 2 digit string, adding a leading 0 for single digits. It is working great, but there has to be DRY way of doing the same thing. Also, the timezone formatting is a bit of a mess. After struggling to come up with a more elegant solution, I just went with what was obvious and working.

def rfc_3339_str(minutes = 0, year = 1999, month = 1, date = 1, secs = 0, time_zone = 0):
    '''Takes minutes and builds RFC 3339 timeDate string.  
        yyyy-mm-ddTHH:MM:ss[.mmm][+/-HH:MM].  Does not support milliseconds.  
        All params optional.  Timezone format is number in range 
        [-12:12] (hours only).  All numbers expect integers.
        Minutes > 1439 (23 hours 59 minutes) wrap around to zero,.
        Date defaults to 1/1/1999.  Supported years 1000-2035 CE.  Date allows 
        range 0-31 inclusive for all months.  Negative times are not supported.  
        Incorrect params return error description'''
    plus = '+'
    minus = '-'
    date_time_split = 'T'
    date_split = '-'
    time_split = ':'
    time_zone_mins = ':00'
    MAX_MINUTES = 1440
    yyyy = None
    mm = None
    dd = None
    HH = None
    MM = None
    SS = None
    zone_sign = plus
    zz = None

    #validate inputs
    if 1000 > year > 2035:
        return 'year_out_of_range'
    else:
        yyyy = str(year)

    if month in range(1,10):
        mm = '0' + str(month)
    elif month in range(10,13):
        mm = str(month)
    else:
        return 'month_out_of_range'

    if date in range(1, 10):
        dd = '0' + str(date)
    elif date in range(10, 32):
        dd = str(date)
    else:
        return 'date_out_of_range'

    if minutes < 0:
        return 'minutes_out_of_range'
    while minutes > MAX_MINUTES:
        minutes = minutes - MAX_MINUTES
    hours_int = int(minutes / 60)
    mins_int = int(minutes % 60)

    if hours_int in range(0,10):
        HH = '0' + str(hours_int)
    elif hours_int in range(10, 100):
        HH = str(hours_int)
    else:
        return 'hours_out_of_range'

    if mins_int in range(0,10):
        MM = '0' + str(mins_int)
    elif mins_int in range(10,60):
        MM = str(mins_int)
    else:
        return 'minutes_out_of_range'

    if secs in range(0, 10):
        ss = '0' + str(secs)
    elif secs in range(10,60):
        ss = str(secs)
    else:
        return 'seconds_out_of_range'

    small = False
    positive = False
    if time_zone in range(-9, 10):
        small = True
    if time_zone in range(-12,0):
        positive = False
    if time_zone in range(0, 13):
        positive = True

    if small:
        zz = '0'
    if positive:
        zz = plus + zz + str(time_zone)
    if not positive:
        zz = minus + zz + str(abs(time_zone))
    else:
        zz = plus + '00'

    print(yyyy + date_split + mm + date_split + dd + 
        date_time_split + HH + time_split + 
        MM + time_split + ss + 
        zz + time_zone_mins)
share|improve this question
    
You can save yourself all the validation hassle by just taking a datetime as an argument. –  georg Aug 20 at 22:10

3 Answers 3

First off, I would like to point you to the time module which has functions that are similar to what you want to accomplish.

Now as for DRYing your code up a bit. You were right to notice that you have blocks of:

if var in range1:
    func1(var)
elif var in range2:
    func2(var)
else:
    return 'blah_out_of_range'

I don't agree with your else block code. You are returning an error statement, which should actually be an exception such as a ValueError like so:

raise ValueError("blah out of range")

This is more readable, and more Pythonic.

Back to the code block, if we take each of the variant parts as arguments into a super function we get:

def process(var, cond1, func1, cond2, final):
    # ugly code here

This is trying to emulate the functional programming style of Lisp, Scheme, Racket and etc, which something that is generally avoided in Python code due to the final unreadability of the code.

Let's think of something else, such as checking all the conditions ahead of time, before doing any processing. This speeds up execution for failed inputs (as any failing input would fail as fast as possible), and simplifies the logic.

This would look something like this:

if year not in range(1001, 2035):
    raise ValueError("year not in range")
if month not in range(1, 13):
    raise ValueError("month not in range")

and so on. This can also be done via assert statements of the unittest module for even more compact code.

Then afterwards you will have blocks of the form:

if cond1:
    func1
else:
    func2

This is easily wrapped into a ternary operator:

func1 if cond1 else func2

Which will eliminate a lot of lines from your code.

share|improve this answer
    
Thank you for looking through this! Could you also address adding a 0 char in front of single digit ints? I can imagine setting flags during validation and inserting the 0 later based on the flags, or converting the validated ints to strings, then adding a leading 0 based on the length, but those don't sem any better than what i have. Would one final loop to add leading 0s as necessary all at one be better or worse? –  nexus_2006 Aug 20 at 20:24
    
After implementing my suggestions the code will look relatively simple, but I understand the desire to make it even smaller. The one final loop idea might sound great, but it will be some pretty ugly code. Don't bother with the flags idea either, as validation should be just to check bounds, nothing else. Your middle code should all be something like mm = '0' + str(month) if month in range(1, 10) else str(month) which is pretty compact. –  mleyfman Aug 20 at 20:28
    
As, @vnp pointed out, that segment of code could be mm = "%02d" % month, so that is about as short as you can make it. –  mleyfman Aug 20 at 21:13

Python supports C-style string formatting:

mm = "%02d" % month

achieves your goal in just one line.

share|improve this answer
    
This still requires range checking, as this code gives you nonsense for month = 25 –  mleyfman Aug 20 at 21:12
    
Correct. My comment addresses only formatting logic. –  vnp Aug 20 at 21:28

Thanks to answers from @vnp and @mleyfman, here is my suggestion. Down to 50 lines (formerly 95). While reviewing python string formatting, I found the String.zfill function, which appears to accomplish the same thing. This made it practical to support years of 1 to 4 digits, so functionality is expanded and docstring updated.

Timezone checking can be simplified with one basic range check. positive and small flags replaced with a more clear ternary switch to check for positive/negative sign while formatting, combined with zfill.

def rfc_3339_str(minutes = 0, secs = 0, year = 1999, month = 1, date = 1, time_zone = 0):
    '''Takes minutes and builds RFC 3339 timeDate string.  
        yyyy-mm-ddTHH:MM:ss[.mmm][+/-HH:MM].  Does not support milliseconds.  
        All params optional.  Timezone format is number in range 
        [-12:12] (hours only).  All numbers expect integers.
        Minutes > 1439 (23 hours 59 minutes) wrap around to zero,.
        Date defaults to 1/1/1999.  Supported years 1 CE - 2035 CE.  Date allows 
        range 0-31 inclusive for all months.  Negative times are not supported.  
        Incorrect params not guaranteed to cause errors (eg 5 digit years).'''
    plus = '+'
    date_time_split = 'T'
    date_split = '-'
    time_split = ':'
    time_zone_mins = ':00'
    MAX_MINUTES = 1440

    #validate inputs
    if year not in range(0, 2036):
        raise ValueError('year out of range')
    if month not in range(1,13):
        raise ValueError('month out of range')
    if date not in range(1, 32):
        raise ValueError('date out of range')
    if minutes < 0:
        raise ValueError('minutes out of range')
    while minutes > MAX_MINUTES:
        minutes = minutes - MAX_MINUTES
    hours_int = int(minutes / 60)
    mins_int = int(minutes % 60)
    if hours_int not in range(0,24):
        raise ValueError('hours out of range')
    if mins_int not in range(0,60):
        raise ValueError('minutes out of range')
    if secs not in range(0, 60):
        raise ValueError('seconds out of range')
    if time_zone not in range(-12, 13):
        raise ValueError('time zone out of range')

    #validation complete, format strings
    yyyy = str(year).zfill(4)
    mm = str(month).zfill(2)
    dd = str(date).zfill(2)
    HH = str(hours_int).zfill(2)
    MM = str(mins_int).zfill(2)
    ss = str(secs).zfill(2)
    zz = plus + str(time_zone).zfill(2) if time_zone in range(0,13) else str(time_zone).zfill(3)

    print(yyyy + date_split + mm + date_split + dd + 
        date_time_split + HH + time_split + 
        MM + time_split + ss + 
        zz + time_zone_mins)
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.