7
\$\begingroup\$

A date is un-ambiguous when each number may be decided to be day or month or year given the range limitation of these numbers.

For example

  • \$14\$ cannot be a month number
  • \$2008\$ cannot be a day number

@param un_ambiguous_date: an un-ambiguous scrambled date, of the format X/Y/Z where X, Y, Z correspond to day, month, year but not necessarily in this order.

@return The date in the order 'DAY/MONTH/YEAR'.

I am particularly interested in full input validation and nice error producing, so tell me any lack of clarity/corner case missed.

For example usage see the doctests included with the function:

def order_date(un_ambiguous_date): """ @param un_ambiguous_date: an un-ambiguous scrambled date, of the format X/Y/Z where X, Y, Z correspond to day, month, year but not necessarily in this order. @return: The date in the order 'DAY/MONTH/YEAR'. A date is un-ambiguous when each number may be decided to be day or month or year given the range limitation of this numbers. (For example 14 cannot be a month number, and 2008 cannot be a day number) >>> order_date('3/2015/13') '13/3/2015' >>> order_date('2012/20/4') '20/4/2012' # Here both 3/4/1000 and 4/3/1000 are possible. >>> order_date('3/4/1000') Traceback (most recent call last): ... ValueError: Ambiguous date was given >>> order_date('3/3/2050') '3/3/2050' >>> order_date('1/1/1') Traceback (most recent call last): ... ValueError: The date cannot be valid >>> order_date('12/6') Traceback (most recent call last): ... ValueError: The date is too short, format should be X/Y/Z >>> order_date('2000/2000') Traceback (most recent call last): ... ValueError: The date is too short, format should be X/Y/Z >>> order_date('Foo/Bar/Baz') Traceback (most recent call last): ... TypeError: The date should be made up of '/' separated INTEGERS """ try: x, y, z = un_ambiguous_date.split('/') except ValueError: raise ValueError("The date is too short, format should be X/Y/Z") try: x, y, z = map(int, (x, y, z)) except ValueError: raise TypeError("The date should be made up of '/' separated INTEGERS") day = [i for i in (x,y,z) if i < 31] month = list(set(i for i in (x,y,z) if i < 12)) year = [i for i in (x,y,z) if i > 31] day = list(set(day) - set(month)) if not day: day = month # print(day, month, year) if any(len(x) > 1 for x in (day, month, year)): raise ValueError("Ambiguous date was given") try: return '/'.join(map(str, (day[0], month[0], year[0]))) except IndexError: raise ValueError("The date cannot be valid") 
\$\endgroup\$
6
  • \$\begingroup\$ What about year values from the dark ages like 28? \$\endgroup\$ Commented Dec 19, 2015 at 19:50
  • \$\begingroup\$ @πάνταῥεῖ For simplicity I assume more recente dates, people do not give precise dates to that periodo anyway \$\endgroup\$ Commented Dec 19, 2015 at 20:31
  • \$\begingroup\$ Simplicity is one of the devils and dumb peoples preferred tools ;-P ... If people ask for more recent years, they would like to omit the 20 anyways, which renders the problem even worse. \$\endgroup\$ Commented Dec 19, 2015 at 20:34
  • 1
    \$\begingroup\$ There is nothing "simple" about dealing with dates. I think we need someone with a strong CS background to reinvent the calendar. \$\endgroup\$ Commented Dec 19, 2015 at 21:45
  • 2
    \$\begingroup\$ In case you're not aware, unambiguous is a single word, so reading the variable name as un_ambiguous_word instead of unambiguous_word was sort of odd for me... \$\endgroup\$ Commented Dec 19, 2015 at 22:33

3 Answers 3

6
\$\begingroup\$

Wrong bounds

Your filters on your day/month aren't right:

day = [i for i in (x,y,z) if i < 31] month = list(set(i for i in (x,y,z) if i < 12)) 

31 is a valid day and 12 is a valid month, so those should be <=. Additionally, you're not excluding negative numbers - so you'll want 1 <= i <= 31 for the first filter and 1 <= i <= 12 for the second.

Dealing with the tuple

Since just about everywhere you deal with (x,y,z) as a tuple, I would just keep it as a tuple the whole way and not unpack it. You only unpack it to check the length, so we can just make that check explicit. Also, if I passed in something like 1/2/3/4/5, you'd tell me that my date was too short, so this can get both right:

elems = un_ambiguous_date.split('/') if len(elems) != 3: raise ValueError("The date is too {}, format should be X/Y/Z".format( 'short' if len(elems) < 3 else 'long')) try: elems = map(int, elems) except ValueError: raise ... day = [i for i in elems if 1 <= i <= 31] # etc. 

The last join

Instead of:

return '/'.join(map(str, (day[0], month[0], year[0]))) 

I would just use format:

return '{}/{}/{}'.format(day[0], month[0], year[0]) 

It's shorter and more readable.

Combine the last two checks

I would write a helper function that returns the first element of a single-element list:

def front(lst): if not lst: raise ValueError("The date cannot be valid") if len(lst) > 1: raise ValueError("Ambiguous date was given") return lst[0] 

And then you can just write:

return '{}/{}/{}'.format(front(day), front(month), front(year)) 
\$\endgroup\$
4
\$\begingroup\$

Another thing you could consider, is to catch the AttributeError if split is called in something else but a string. Of course you could also check for isinstance(un_ambiguous_date, str), but catching the attribute error fits the rest of the code better.

With the information about your focus on 'recent dates' in the comments, you could also introduce an additional constraint that at least one of the parts (supposedly the year), should have four digits (keep this in mind if you plan to deploy your program beyond the year 9999).

\$\endgroup\$
1
  • \$\begingroup\$ I guess it would be nicer to use static typing for this instead of isinstance(un_ambiguous_date, str) but I do not usually strive for types in dynamicly typed languages anyway \$\endgroup\$ Commented Dec 19, 2015 at 23:13
1
\$\begingroup\$
  • run your tests, your doctests do not pass

  • variables x, y, z does add anything in terms of semantic meaning and you are only constructing tuple out of it, so it should be kept just in the list as it is (will also get rid of the first try/except):

    n = un_ambiguous_date.split('/') 
  • IndexError will never happen because of Ambiguous date or The date is too shorterrors, however you should still do the length check if you are not breaking down it to x, y, z:

    if len(n) != 3: raise ValueError("The date cannot be valid") 
  • the part where you break down day, month, year with list comprehensions is kind of verbose. Could be written as

    n.sort() month, day, year = n if not(0 < month <= 12) or not(12 < day <= 31) or year <= 31: raise ValueError("Ambiguous date was given") 
  • last join could be done with format(see @Barry answer) or printf-style string formatting:

    return '%d/%d/%d' % (day, month, year) 
\$\endgroup\$
2
  • \$\begingroup\$ The tests pass now, there was a small typo that I fixed (0 -> 31). Also I suggest a nicer error messageif len(n) != 3: raise ValueError("The date cannot be valid") -> if len(n) != 3: raise ValueError("The date must contain exactly 3 pieces"), I usually use sorted and not .sort()and % formatting is deprecated. Anyhow I like your approach with sorting \$\endgroup\$ Commented Dec 20, 2015 at 10:32
  • \$\begingroup\$ Your solution presents a problem with equal day and month, that is allowed, like order_date('3/3/2050') that should be accepted \$\endgroup\$ Commented Dec 20, 2015 at 10:34

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.