8
\$\begingroup\$

I am a high school math and science teacher. I am working with my students on the concepts of algorithms, unit conversions, and functions. In class, to work on all these concepts, we just worked out the process of how to calculate how old someone is in seconds. I thought this was a particularly interesting algorithm to feed to a computer, so I spent the next couple of days working on this in my spare time.

I have not sanitized my user input or done any internal error checking, and the code assumes they were born at exactly 00:00:00 on their day of birth. Laziness on my party, I guess. I am most concerned with "Is there a better way to calculate my days offset?" This is either the number of days until their birthday this year or since their birthday this year. There is a specific function or three for this, as you can see below in the code.

Additionally, are there any bits of particularly cringy (my term) code?

I had to google how to get current date and time ("c++ date time", first link). So I totally just stole that bit of code.

Thank you very much. I tried to comment clearly, but occasionally I might have been less commenty than desirable.

/*
 * For use by absolutely anyone for absolutely any reason.
 */

/* 
 * File:   main.cpp
 * Author: Wayman Bell III
 *
 * Created on March 31, 2016
 */

#include <iostream>
#include <ctime>

using namespace std;

int getTheYear();
int getTheMonth();
int getTheDay();
int getTheHour();
int getTheMinute();
int getTheSecond();
int countLeapYears(int, int);
int welcome();
int calcAgeInSeconds(int, int, int);
int calcOffset(int, int);
int calcDaysRemainingThisYear(int, int);
int calcDaysSinceBDay(int, int);

int main(int argc, char** argv) 
{

    welcome();
    return 0;
}

int getTheYear( ) //Retrieve current year
{
   // current date/time based on current system
   time_t now = time(0);

   tm *ltm = localtime(&now);

   return (1900+ltm->tm_year); // print various components of tm structure.
   /*cout << "Year: "<< 1900 + ltm->tm_year << endl;*/
}

int getTheMonth() //Retrieve current month
{
    // current date/time based on current system
   time_t now = time(0);

   tm *ltm = localtime(&now);

   return (1+ltm->tm_mon); // print various components of tm structure.
   /*cout << "Month: "<< 1 + ltm->tm_mon<< endl;*/
}

int getTheDay() //Retrieve current day of month
{
    // current date/time based on current system
   time_t now = time(0);

   tm *ltm = localtime(&now);

   return (ltm->tm_mday); // print various components of tm structure.
   /*cout << "Day: "<<  ltm->tm_mday << endl;*/
}

int getTheHour() //Retrieve current hour
{
    // current date/time based on current system
   time_t now = time(0);

   tm *ltm = localtime(&now);

   return (1+ltm->tm_hour); // print various components of tm structure.
   /*cout << "Time: "<< 1 + ltm->tm_hour << ":";*/
}

int getTheMinute() //Retrieve current minute
{
    // current date/time based on current system
   time_t now = time(0);

   tm *ltm = localtime(&now);

   return (1+ltm->tm_min); // print various components of tm structure.
   /*cout << 1 + ltm->tm_min << ":";*/
}

int getTheSecond() //Retrieve current second
{
    // current date/time based on current system
   time_t now = time(0);

   tm *ltm = localtime(&now);

   return (1+ltm->tm_sec); // print various components of tm structure.
   /*cout << 1 + ltm->tm_sec << endl;*/
}

int welcome() //Get birthday input from user.  Pass info to appropriate functions. Output age in seconds.
{
    std::cout << "Welcome to the Age Calculator!\n";
    std::cout << "January \t-- 1\t|\tFebruary \t-- 2\nMarch \t\t-- 3\t|\tApril \t\t-- 4\n";
    std::cout << "May \t\t-- 5\t|\tJune \t\t-- 6\nJuly \t\t-- 7\t|\tAugust \t\t-- 8\n";
    std::cout << "September \t-- 9\t|\tOctober \t-- 10\nNovember \t-- 11\t|\tDecember \t-- 12";
    std::cout << "\n\nWhat month were you born in: ";

    int monthBorn=0;
    std::cin >> monthBorn;

    int dayOfMonthBorn=0;
    std::cout << "Enter the day of the month you were born: ";
    std::cin >> dayOfMonthBorn;

    int yearBorn=0;
    std::cout << "Enter the year you were born: ";
    std::cin >> yearBorn;
    std::cout << "\nThank you.  One moment.\nCalculating...\n\n";

    int ageInSeconds=0;
    ageInSeconds=calcAgeInSeconds(monthBorn,dayOfMonthBorn,yearBorn);

    std::cout << "You are " << ageInSeconds << " seconds old! Congratulations!\n\n";
    std::cout << "Enter \"0\" to end. ";

    char endPrg = ' ';
    std::cin >> endPrg;

    return 0;
}

int calcAgeInSeconds(int month, int day, int year)
{
   int curYear  =   getTheYear();
   int curMon   =   getTheMonth();
   int curDay   =   getTheDay();
   int curHour  =   getTheHour();
   int curMin   =   getTheMinute();
   int curSec   =   getTheSecond();
   int leapCount=   countLeapYears(curYear, year);
   int yearsOld =   curYear - year;

   //If person has not had their birthday yet, they are a year younger.
   if (curMon < month || (curMon == month && curDay < day))
       yearsOld--;

   //If this is a leap year, but before leap day, then subtract one leap day.
   if (((curYear / 4) * 4 == curYear) && (curMon < 2 || (curMon == 2 && curDay <= 28)))
       leapCount--;

   //If person born on leap year, but after leap day, subtract one leap day.
   if (((year / 4) * 4 == year) && (month > 2))
       leapCount--;

   int secondsOld   = 0;
   int dayOffset    = 0;
   dayOffset        = calcOffset(month, day); //Account for days since birthday or until birthday
   secondsOld       = yearsOld * 365 + leapCount + dayOffset; //Add up total number of days
   secondsOld       = secondsOld * 24 + curHour; //Convert to hours and add today's hours.
   secondsOld       = secondsOld * 60 + curMin; //Convert to minutes and add today's minutes.
   secondsOld       = secondsOld * 60 + curSec; //Convert to seconds and add today's seconds.

   return secondsOld; 
}

int calcOffset(int theirMon, int theirDay)
{
   int curYear  =   getTheYear();
   int curMon   =   getTheMonth();
   int curDay   =   getTheDay();
   int curHour  =   getTheHour();
   int curMin   =   getTheMinute();
   int curSec   =   getTheSecond(); 
   int dayOff   =   0;

   //If they have not yet had their birthday...
   if ((curMon < theirMon) || ((curMon == theirMon) && (curDay < theirDay)))
       dayOff = 365 - calcDaysRemainingThisYear(theirMon, theirDay);
   //If they have had their birthday...
   else if ((curMon == theirMon) && (curDay == theirDay))
       dayOff = 0;
   else
       dayOff = calcDaysSinceBDay(theirMon, theirDay);

   return dayOff;
}

int calcDaysRemainingThisYear(int theirMon, int theirDay)
{
    int curYear     = getTheYear();
    int curDay      = getTheDay();
    int curMon      = getTheMonth();
    int dayOffset   = 0;

    while (curMon < theirMon)
    {
        if (curMon == 1)
            dayOffset += 31;
        else if (curMon == 2)
        {
            if ((curYear / 4) * 4 == curYear)
                dayOffset += 29;
            else dayOffset += 28;
        }
        else if (curMon == 3)
            dayOffset += 31;
        else if (curMon == 4)
            dayOffset += 30;
        else if (curMon == 5)
            dayOffset += 31;
        else if (curMon == 6)
            dayOffset += 30;
        else if (curMon == 7)
            dayOffset += 31;
        else if (curMon == 8)
            dayOffset += 31;
        else if (curMon == 9)
            dayOffset += 30;
        else if (curMon == 10)
            dayOffset += 31;
        else if (curMon == 11)
            dayOffset += 30;
        else if (curMon == 12)
        {
            dayOffset += 31;
            curMon = 0;
        }

        curMon ++;
    }

    dayOffset -= curDay;
    dayOffset += theirDay;

    return dayOffset;
}

int calcDaysSinceBDay(int theirMon, int theirDay)
{
    int curYear     = getTheYear();
    int curDay      = getTheDay();
    int curMon      = getTheMonth();
    int dayOffset   = 0;

    while (theirMon < curMon)
    {
        if (theirMon == 1)
            dayOffset += 31;
        else if (theirMon==2)
        {
            if (((curYear - 1) / 4) * 4 == (curYear - 1))
                dayOffset += 29;
            else dayOffset += 28;
        }
        else if (theirMon == 3)
            dayOffset += 31;
        else if (theirMon == 4)
            dayOffset += 30;
        else if (theirMon == 5)
            dayOffset += 31;
        else if (theirMon == 6)
            dayOffset += 30;
        else if (theirMon == 7)
            dayOffset += 31;
        else if (theirMon == 8)
            dayOffset += 31;
        else if (theirMon == 9)
            dayOffset += 30;
        else if (theirMon == 10)
            dayOffset += 31;
        else if (theirMon == 11)
            dayOffset += 30;
        else if (theirMon == 12)
        {
            dayOffset += 31;
            theirMon = 0;
        }

        theirMon ++;
    }

    dayOffset -= theirDay;
    dayOffset += curDay;

    return dayOffset;
}

int countLeapYears(int curYear, int theirYear)
{
    //Find nearest year divisible by 4 beginning at or prior to the start count,
    //then begin subtracting 4 from the start count until we get to the finish count
    int leapYears   =   0;
    int modYears    =   0;

    //Found a better way than the comment under this code.
    //No If statements required, just find the mod, take it out, calculate leaps.
    modYears = curYear % 4;
    curYear -= modYears;
    while (curYear >= theirYear)
    {
        curYear -= 4;
        leapYears++;
    }

    return leapYears;
    /* Found a better way than this.  See above.
    if ((curYear / 4) * 4 == curYear) //This year is a leap year.
    {

    }
    else if (((curYear - 1) / 4) * 4 == curYear) //Last year was a leap year.
    {
        while ((curYear - 1) >= theirYear)
        {
            curYear -= 4;
            leapYears++;
        }
    }
    else if (((curYear - 2) / 4) * 4 == curYear) //Year before last was a leap year.
    {
        while ((curYear - 2) >= theirYear)
        {
            curYear -= 4;
            leapYears++;
        }
    }
    else //Next year is a leap year.
    {
        while ((curYear - 3) >= theirYear)
        {
            curYear -= 4;
            leapYears++;
        }
    }

    return leapYears;*/
}
\$\endgroup\$
  • \$\begingroup\$ Excellent job on your first question. \$\endgroup\$ – user34073 Apr 6 '16 at 4:13
  • 1
    \$\begingroup\$ I'm too tired to write a full review (it's about 11:30PM where I'm at), but a few things I do see at a first glance are the unnecessary function signatures at the top, the inclusion of using namespace std;, the return 0; in main and the unnecessary inclusion of command line argument parameters in main. These can all be removed. \$\endgroup\$ – Ethan Bierlein Apr 6 '16 at 4:19
  • 1
    \$\begingroup\$ Just as a heads up. If this was a question at an interview I would expect you to convert the dates into seconds from some epoc then subtract them (that''s the easy way to get the difference between two dates). \$\endgroup\$ – Martin York Apr 6 '16 at 7:38
  • \$\begingroup\$ @LokiAstari that is an excellent suggestion. I am not at all familiar with ctime, and I had a difficult time understanding even the example I found in my google search, but I suspect that ctime can do most everything I wanted in this program much more simply. As I understand it, the initial call for time from ctime actually does return the number of seconds since epoch (1900? 1970?). I don't fully understand how ctime is defining an epoch. \$\endgroup\$ – Wayman Bell III Apr 6 '16 at 17:02
  • \$\begingroup\$ I wouldn't do that by myself. Instead, I would use the stuff here throughout. \$\endgroup\$ – davidhigh Apr 6 '16 at 21:15
7
\$\begingroup\$
using namespace std;

Don't do this, compare for example Why is “using namespace std” in C++ considered bad practice?. And actually your code does not rely on it, you can simply remove that line.


As already mentioned in the comments, neither the argc/argv parameters nor the return statement is required in C++ (see e.g. What should main() return in C and C++? for an overview):

int main() 
{
    welcome();
}

Your welcome() function does all the work (which makes the function name quite misleading). A better design would be to separate between input, calculation, and output:

void askForBirthdate(int &dayOfBirth, int &monthOfBirth, int &yearOfBirth)
{
    // ...
}

int main()
{
    int dayOfBirth, monthOfBirth, yearOfBirth;
    askForBirthdate(dayOfBirth, monthOfBirth, yearOfBirth);
    int ageInSeconds = calcAgeInSeconds(dayOfBirth, monthOfBirth, yearOfBirth);
    std::cout << "You are " << ageInSeconds << "old.\n";
}

(This will be revised below.)


Your age calculation is quite complicated and has errors. As pointed out in the other answers:

  • The leap year algorithm is not correct.
  • Daylight save time transitions are not considered.
  • The current time is retrieved multiple times which can cause inconsistent results.

Instead of converting the current time to day/month/year/... and computing the difference to the birth day/month/year, it would be easier to go the other way around: Convert the birth date to a time value (seconds since Jan 1, 1970 UTC) and compute the difference to the current time value (which is obtained by time(0).

Converting day/month/year to a time value (according to the local timezone) is easily done with mktime() (the "counterpart" to localtime()):

time_t timeForDate(int day, int month, int year)
{
    struct tm timeinfo = { 0 };
    timeinfo.tm_mday = day;
    timeinfo.tm_mon = month - 1;
    timeinfo.tm_year = year - 1900;
    timeinfo.tm_isdst = -1;
    return mktime(&timeinfo);
}

The main program then becomes

int main()
{
    int dayOfBirth, monthOfBirth, yearOfBirth;
    askForBirthdate(dayOfBirth, monthOfBirth, yearOfBirth);

    time_t birthTime = timeForDate(dayOfBirth, monthOfBirth, yearOfBirth);
    time_t nowTime = time(0);
    time_t ageInSeconds = nowTime - birthTime;

    std::cout << "You are " << ageInSeconds << " seconds old.\n";
}

Some other miscelleaneous remarks:

  • You can get rid of the function declarations if you define all functions before using them.

  • Statements like

    int ageInSeconds=0;
    ageInSeconds=calcAgeInSeconds(monthBorn,dayOfMonthBorn,yearBorn);
    

    can be combined to

    int ageInSeconds = calcAgeInSeconds(monthBorn, dayOfMonthBorn, yearBorn);
    

    And use more (horizontal) space!

  • Check and fix all compiler warnings. For example, in calcOffset(), curYear and three more variables are computed but their values are never used.

  • Always use curly braces { } with if-statements, even if the if or else part consists only of a single statement. That helps to avoid errors if the code is edited later.

  • The long if/else if/else if/... statement in calcDaysRemainingThisYear() can be simplified by using a switch statement.

\$\endgroup\$
  • \$\begingroup\$ Wow, I am blow away by the depth of your feedback. Thank you very much for having such a thorough look at my code. I was taught to initialize variables before using them. It always seemed silly to me if I was going to immediately then load the desired value into them, but I thought it must be a best practices thing. Same with my function declarations. If I am not mistaken, you are saying that as long as I code my functions in order from "Last possible function called" to "First possible function called" vertically, I will be okay? I have more questions about your feedback for later. \$\endgroup\$ – Wayman Bell III Apr 6 '16 at 23:23
  • \$\begingroup\$ @WaymanBellIII: You are welcome! – Yes, that is correct. \$\endgroup\$ – Martin R Apr 7 '16 at 5:13
8
\$\begingroup\$

Non-atomic time problem

I just wanted to point out one thing. You currently have a few places where you do this:

int curYear  =   getTheYear();
int curMon   =   getTheMonth();
int curDay   =   getTheDay();
int curHour  =   getTheHour();
int curMin   =   getTheMinute();
int curSec   =   getTheSecond();

Each of these functions gets the current time using time(0) and then extracts the particular aspect of the time. However, this is unsafe because if you happen to run this code at a time where a wraparound occurs, you could get inconsistent results for your 6 variables.

For example, consider what happens if you ran the code at 12/31/2015 23:59:59, and while you were getting the 6 variables, the time incremented by one second. You could end up thinking the date was one of these 5 possibilities, all of which are wrong by varying degrees:

 1/ 1/2015 00:00:00 (Off by one year  )
12/ 1/2015 00:00:00 (Off by one month )
12/31/2015 00:00:00 (Off by one day   )
12/31/2015 23:00:00 (Off by one hour  )
12/31/2015 23:59:00 (Off by one minute)

To fix this, you should read the time once, and then pass the time into your functions:

static inline int getTheYear(const tm *ltm)
{
    return 1900 + ltm->tm_year;
}

static inline int getTheMonth(const tm *ltm)
{
    return 1 + ltm->tm_mon;
}

// ...

{
    time_t now      = time(0);
    tm    *ltm      = localtime(&now);
    int    curYear  = getTheYear(ltm);
    int    curMon   = getTheMonth(ltm);
    // ...
}
\$\endgroup\$
  • \$\begingroup\$ Hey, I wanted to say thank you for your input and analysis. Your illumination of the potential errors with my time function has been very helpful. I am re-writing the code to try to implement your suggestion. Thank you! :) \$\endgroup\$ – Wayman Bell III Apr 12 '16 at 1:35
3
\$\begingroup\$

Just one aspect: You treat every year which is divisible by 4 as a leap year. That is not correct. The rules are (for the Gregorian calendar)

Every year that is exactly divisible by four is a leap year, except for years that are exactly divisible by 100, but these centurial years are leap years if they are exactly divisible by 400. For example, the years 1700, 1800, and 1900 are not leap years, but the year 2000 is.

(from https://en.wikipedia.org/wiki/Leap_year#Gregorian_calendar)

A correct method to determine if a year is a leap year would therefore be

bool isLeapYear(int year)
{
    return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
}

Another problem is that transitions from and to daylight savings time are not taken into account in your code (as far as I can see).

\$\endgroup\$

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.