6
\$\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\$
  • \$\begingroup\$ What about year values from the dark ages like 28? \$\endgroup\$ – πάντα ῥεῖ Dec 19 '15 at 19:50
  • \$\begingroup\$ @πάνταῥεῖ For simplicity I assume more recente dates, people do not give precise dates to that periodo anyway \$\endgroup\$ – Caridorc Dec 19 '15 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\$ – πάντα ῥεῖ Dec 19 '15 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\$ – Barry Dec 19 '15 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\$ – Schism Dec 19 '15 at 22:33
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\$
  • \$\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\$ – Caridorc Dec 19 '15 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\$
  • \$\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\$ – Caridorc Dec 20 '15 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\$ – Caridorc Dec 20 '15 at 10:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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