2
\$\begingroup\$

This is a code to calculate somebody's age. It takes 2 inputs:

  • A specific date (to calculate your age at a specific time or the current time).
  • The birth date.
print('''# This is Age Calculator Program.
# This program is written by Mahmoud Naguib and it will be improved.
# You can contact me at : https://www.facebook.com/naguibarea
--------------------------------------------------------------------''')
import re

while True:
    spefic_date = input('Enter a spefic date (like this format: day/month/year): ')
    birth_date = input('Enter the birth date (like this format: day/month/year): ')
    print('--------------------------------------------------------------------')
    # This line makes sure that the user's inputs are correct
    # if not it will print a message and the user will enter them again.
    if re.search(r'\d{1,2}/\d{1,2}/\d{4}', spefic_date) is None or re.search(r'\d{1,2}/\d{1,2}/\d{4}', birth_date) is None:
        print('You have entered a wrong format! ')
        print('--------------------------------------------------------------------')
        continue
    # This will convert the user's input into lists to use them in assigning values.
    spefic_date = spefic_date.split('/')
    birth_date = birth_date.split('/')

    # This line assigns spefic year's, month's and day's values.
    spefic_year, spefic_month, spefic_day = int(spefic_date[2]), int(spefic_date[1]), int(spefic_date[0])
    # This line specifies birth year's, month's and day's values.
    birth_year, birth_month, birth_day = int(birth_date[2]), int(birth_date[1]), int(birth_date[0])
    # These lines are for math rules.
    if spefic_day < birth_day:
        spefic_day += 30
        spefic_month -= 1
    if spefic_month < birth_month:
        spefic_month += 12
        spefic_year -= 1
    # These lines calculate years, months and days.  
    year = str(spefic_year - birth_year) + ' years, '
    month = str(spefic_month - birth_month) + ' months and '
    day = str(spefic_day - birth_day) + ' days. '
    # These lines are for grammar rules.
    if spefic_year - birth_year < 2:
        year = year.replace('years', 'year')
    if spefic_month - birth_month < 2:
        month = month.replace('months', 'month')
    if spefic_day - birth_day < 2:
        day = day.replace('days', 'day')
    print('Your age is: ' + year + month + day)
    print('--------------------------------------------------------------------')
\$\endgroup\$
0

3 Answers 3

3
\$\begingroup\$

"spefic" changed to "specific" everywhere

specific_date = specific_date.split('/')
birth_date = birth_date.split('/')

# This line assigns specific year's, month's and day's values.
specific_year, specific_month, specific_day = int(specific_date[2]), int(specific_date[1]), int(specific_date[0])
# This line specifies birth year's, month's and day's values.
birth_year, birth_month, birth_day = int(birth_date[2]), int(birth_date[1]), int(birth_date[0])

You could use map to convert each of the date values to an integer:

specific_date = map(int, specific_date.split('/'))
birth_date = map(int, birth_date.split('/'))

# This line assigns specific year's, month's and day's values.
specific_year, specific_month, specific_day = specific_date[2], specific_date[1], specific_date[0]
# This line specifies birth year's, month's and day's values.
birth_year, birth_month, birth_day = birth_date[2], birth_date[1], birth_date[0]

Also there's no need to index these date iterables to unpack them. You can simply unpack them directly because they are iterable:

specific_date = map(int, specific_date.split('/'))
birth_date = map(int, birth_date.split('/'))
specific_year, specific_month, specific_day = specific_date
birth_year, birth_month, birth_day = birth_date

I think the specific_ and birth_ prefixes make it a little difficult to see what is going on in the code. I would use shorter variable names that make it easier to line up like values.

year1, month1, day1 = map(int, birth_date.split('/'))
year2, month2, day2 = map(int, specific_date.split('/'))

Even better would be to use a namedtuple (thanks @holroy for the suggestion).

You'll need to import namedtuple first:

from collections import namedtuple

Change to:

Date = namedtuple('Date', 'year month day')
birthday = Date(*map(int, birth_date.split('/')))
today = Date(*map(int, specific_date.split('/')))

I'm not certain the arithmetic in your code is correct.

As for the strings that are printed. I would use string formatting instead of concatenation and I would avoid replace.

year_word = 'year' if today.year - birthday.year < 2 else 'years'
month_word = 'month' if today.month - birthday.month < 2 else 'months'
day_word = 'day' if today.day - birthday.day < 2 else 'days'
age = '{years} {year_word}, {months} {month_word} and {days} {day_word}. '.format(
    years=(today.year - birthday.year),
    month=(today.month - birthday.month),
    day=(today.day - birthday.day),
    year_word=year_word,
    month_word=month_word,
    day_word=day_word,
)
print('Your age is: {age}'.format(age=age))
print('--------------------------------------------------------------------')
\$\endgroup\$
4
  • 1
    \$\begingroup\$ You've got a bug in your plural/singular detection where you've switched which is larger. The y1 vs y2 should be switched around either in the year/years statement, or within the format. The same applies for the other variables. In addition using shorter names, really hinders the readability. \$\endgroup\$
    – holroy
    Nov 20, 2015 at 11:07
  • \$\begingroup\$ Good catch on the swapped variables names and very good point on the short variable names hindering readability. I have a problem with specific_year and birth_year because it seems like the variable would be more readable with year first. I made the names more verbose and swapped the numbering so 1 is in the past (birth) and 2 is in the future (specific). \$\endgroup\$ Nov 21, 2015 at 0:05
  • 1
    \$\begingroup\$ You could use a named tuple, so that you reference specific_date.year, or birth_date.month. Having 1 or 2 in a variable name, is a code smell. In my opinion, they should either be in an array/list or have other better names. \$\endgroup\$
    – holroy
    Nov 21, 2015 at 0:09
  • \$\begingroup\$ That's a brilliant idea! Updating now. \$\endgroup\$ Nov 21, 2015 at 0:12
4
\$\begingroup\$

I don't have much time for the review so I'll focus on regular expressions for the time being.

About re

  • You should probably be using match instead of search because you want the pattern to be at the beginning of the string. Read more

  • If you are re-using the same regexp multiple times like you are doing here, it might be worth compile-ing it.

  • You could use the logic used for matching to split your result as well. If you do so, you do not need to change things in multiple places if the format changes;

Like this :

>>> date = "14/07/1789"
>>> m = re.match(r'(\d{1,2})/(\d{1,2})/(\d{4})', date)
>>> m.groups()
('14', '07', '1789')
  • Even better, you could use named elements. If the format changes (because your code is used in a different country where dates have a different format for instance), only the regexp will change, the code will still be ok because it will be accessing elements by their name and not by their position.

Like this :

>>> date = "14/07/1789"
>>> m = re.match(r'(?P<day>\d{1,2})/(?P<month>\d{1,2})/(?P<year>\d{4})', date)
>>> m.groups()
('14', '07', '1789')
>>> m.groupdict()
{'month': '07', 'day': '14', 'year': '1789'}

Also, the whole logic converting strings to dates (or to triples (day, month, year)) could be extracted in a function on its own.

You might also decide that you want to reuse already existing tools for this such as datetime.strptime

Better user experience

When I input 2 dates which is usually quite painful, I'd rather have my program to fail if the first date is incorrect rather than having to input the second date to know it.

\$\endgroup\$
3
\$\begingroup\$

The other answers has mentioned good and valid points regarding how to get the different dates, and handling of these. I would like to focus on your date arithmetic, which should be done using date utils to avoid strange cases.

One issue that you've locked all of the months to be 30 days. What about Febrarury with 28 days (when it's not a leap year), or all the months with 31 days?

Similarily adding 12 to the spefic_month does end up with a non-legal month in some cases, although it does somewhat work for when you calculate the difference afterwords. But this kind of arithmetic on dates can lead to various strange results.

You are usually better of using real dates, and doing date calculations using either builtins (like datetime, or external library specialising on this (like labix dateutil.

Some other style comments:

  • Start using functions – The entire code of your script is on the module/file level, and should be within functions to separate concerns and help you focus on what the different parts do.
  • Use the if __name__ == '__main__': pattern – Calling main() from within this if statement allows for your code to be used as a proper module, and with an accompanying get_date_delta(from_date, to_date) you'll have a handy utility function which could be used in other scripts
  • Make the input handling into a function – Given a function like input_date(date_prompt) which returns a proper date, you could avoid the duplication of code related to input, verifying and validating dates. All could be within that one function. It could also convert into a proper date for later handling
  • Let methods return data, not print directly – Functions should usually return something, and not print directly unless it is a specific output function. In your case I would suggest that the calculation returns either a tuple of the (year, month, day) delta, or possibly a string with the date delta, i.e. 4 years, 1 month, 30 days.

Doing the above mentioned changes could lead to a main handling of:

def main():
    """Get two dates, and display the years, months and days between those."""

    birth_date = input_date("Enter birth date (dd/mm/yyyy): ")
    specific_date = input_date("Enter another date (dd/mm/yyyy): ")

    date_delta = get_date_delta(birth_date, specific_date)

    print('Difference from {} to {}: {}'.format(birth_date, specific_date, date_delta)


if __name__ == '__main__':
    main()

Could still need some tweaking, but you get the gist of idea to make it more modular, and to make it read a lot easier, which in turns will make it more maintable.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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