7
\$\begingroup\$

I wrote this little program, but I'm not sure if it's properly written. Actually, it can't handle leap years (every year is 365 days long) and you have to write the earlier date before. It seems to be working correctly.

Can you tell me if it's okay or not, and what should I change in it to make it good?

#include <iostream>
using namespace std;

int first_date_month;
int first_date_days;
int first_date_year;
int second_date_month;
int second_date_days;
int second_date_year;
int days;

int month_days[] = {31,28,31,30,31,30,31,31,30,31,30,31};

int main()
{
    cout << "Enter first date: ";
    cin >> first_date_year >> first_date_month >> first_date_days;
    cout << "Enter second date: ";
    cin >> second_date_year >> second_date_month >> second_date_days;

    if(first_date_year == second_date_year)
    {
        if(first_date_month == second_date_month)
            days = second_date_days - first_date_days;
        else
        {
            for(int i = first_date_month; i < second_date_month-1;i++)
                days += month_days[i];
            days += month_days[first_date_month-1] - first_date_days + second_date_days;
        }
    }
    else
    {
        for(int i = 0; i < second_date_month-1; i++)
            days += month_days[i];
        for(int i = first_date_month; i < 12; i++)
            days += month_days[i];
        if(second_date_year - first_date_year >= 0)
        days += (second_date_year - first_date_year - 1)*365 + 
            month_days[first_date_month-1] - first_date_days + second_date_days;
    }

    cout << "Days between the two dates: " << days;

    return(0);  
}
\$\endgroup\$
  • 2
    \$\begingroup\$ You are doing it the hard way. Convert your dates into unix time stamps (or similar single value ratio). Do a single subtraction then divide by (24*60*60). \$\endgroup\$ – Martin York Aug 29 '13 at 20:17
8
\$\begingroup\$
  • Do not use using namespace std in global scope.

  • Never use global variables. Right away, this will introduce all sorts of problems, including maintainability and bugs. There are different alternatives to this, one being a struct:

    Such a structure just needs the month, day, and year:

    struct Date
    {
        int month;
        int day;
        int year;
    };
    

    Initialize the two Date instances:

    // same order as appears in struct
    Date date1 = {1, 2, 2000};
    Date date2 = {4, 5, 2001};
    

    Access the structures and set the data members (with your code):

    std::cout << "Enter first date: ";
    std::cin >> date1.year >> date1.month >> date1.day;
    std::cout << "Enter second date: ";
    std::cin >> date2.year >> date2.month >> date2.day;
    

    If you want to get into encapsulation/information-hiding, I'd recommend a class instead. If you want to keep this simpler than a struct, just move the globals into main() (but use the variables from my example). You could also create more specialized functions, thereby not just working in main(). Modularity will help keep your code more organized and maintainable.

  • I don't like int for these values (dates cannot be negative). I'd go with std::size_t instead (include <cstddef> to use it).

  • month_days[] should be a const while in global scope. As a constant, it can remain there because it cannot be changed by anything else. However, this will prevent you from accounting for leap-years. Speaking of which...

  • To account for leap-years, I'd either:

    1. leave out February's value from the array (it's the only value that could change)
    2. not make the array a constant (the program will handle the values during runtime)

    With that, you can allow the program to adjust February's value if a leap-year.

\$\endgroup\$
  • \$\begingroup\$ thanks :) so you mean instead of using the namespaces I should use this (std::cout) form? Is it true for every namespace? \$\endgroup\$ – mitya221 Aug 29 '13 at 19:22
  • \$\begingroup\$ Yes. Generally, using is not good in global scope because it can cause name-clashes. For instance, let's say you've created your own pow(), but have also included <cmath>. What happens if you're using using namespace std? The compiler will not be able to tell them apart, causing errors. Plus, you may also not know which is which. Get the idea? :-) That said, you're welcome to put the using in main() because then it won't be in global scope. \$\endgroup\$ – Jamal Aug 29 '13 at 19:27
  • \$\begingroup\$ i see :) thank you :D another question: what if I used a struct like this: struct Date { month; day; year; } and then I create two instances of them (one for first_date, and an other for second_date)? Which solution is better? \$\endgroup\$ – mitya221 Aug 29 '13 at 19:30
  • \$\begingroup\$ Yes, a struct is okay for a starter. I'm already working on an edit regarding that. \$\endgroup\$ – Jamal Aug 29 '13 at 19:31
3
\$\begingroup\$
#include <iostream>
#include <cmath>

int days;


struct Day
{
    int count;
    friend std::istream& operator>>(std::istream& s, Day& d)
    {
        int day_year;
        int day_month;
        int day_days;

        s >> day_year >> day_month >> day_days;

        // calculate number of leap years.
        int leapyears    = day_year / 4;
        if (day_year % 4 == 0 && day_month < 3)
        {
            // If this is a leap year
            // And we have not passed Feburary then it does
            // not count.....
            leapyears   --;
        }
        // convert year/month/day into a day count
        d.count    = day_year * 365 + month_days[day_month-1] + day_days + leapyears;

        // return stream for chaining
        return s;
    }
    friend int operator-(Day const& lhs, Day const& rhs)
    {
        // subtraction gives you the difference between two Days objects.
        return lhs.count - rhs.count;
    }
    static int month_days[];
};

int Day::month_days[] = {0,31,59,90,120,151,181,212,243,273,304,334};

Main is now simple to write:

int main()
{

    // Declare variables as close to the point of first use as you can.
    Day     first;
    std::cout << "Enter first date: ";
    std::cin >> first;

    Day     second;
    std::cout << "Enter second date: ";
    std::cin >> second;

    std::cout << "Days between the two dates: " << std::abs(first - second) << "\n";
}
\$\endgroup\$
  • \$\begingroup\$ thanks :) but it's a bit complicated to me :D I've just started learning c++. \$\endgroup\$ – mitya221 Aug 29 '13 at 21:22
  • \$\begingroup\$ @mitya221: He essentially takes my basic struct suggestion and completes the implementation of the program (using the calculations and such). Of course, feel free to ask specific questions about what you don't understand here. \$\endgroup\$ – Jamal Aug 29 '13 at 21:40
  • \$\begingroup\$ well.. I understand it but I couldn't write such a program yet :) but thank you very much :) \$\endgroup\$ – mitya221 Aug 30 '13 at 10:31
  • \$\begingroup\$ I hate to downvote this, because it's pretty good pedagogic code (apart from failing to check inputs, and getting some leap years wrong). But it's really not a review. \$\endgroup\$ – Toby Speight Oct 31 '18 at 9:53
1
\$\begingroup\$

When reading from a stream, one should always check that the read was successful:

std::cout << "Enter first date: ";
std::cin >> first_date_year >> first_date_month >> first_date_days;
if (!std::cin) {
    std::cerr << "Date format error" << std::endl;
    return 1;
}

(You could be much more helpful with the error message, of course).

It's probably also a good idea to flush the output stream before reading input:

std::cout << "Enter first date: " << std::flush;
\$\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.