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);
}
-
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\$Loki Astari– Loki Astari2013年08月29日 20:17:35 +00:00Commented Aug 29, 2013 at 20:17
3 Answers 3
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 astruct
, just move the globals intomain()
(but use the variables from my example). You could also create more specialized functions, thereby not just working inmain()
. 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 withstd::size_t
instead (include<cstddef>
to use it).month_days[]
should be aconst
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:
- leave out February's value from the array (it's the only value that could change)
- 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.
-
\$\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– mitya2212013年08月29日 19:22:40 +00:00Commented Aug 29, 2013 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 ownpow()
, but have also included<cmath>
. What happens if you're usingusing 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 theusing
inmain()
because then it won't be in global scope. \$\endgroup\$Jamal– Jamal2013年08月29日 19:27:27 +00:00Commented Aug 29, 2013 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– mitya2212013年08月29日 19:30:17 +00:00Commented Aug 29, 2013 at 19:30
-
\$\begingroup\$ Yes, a
struct
is okay for a starter. I'm already working on an edit regarding that. \$\endgroup\$Jamal– Jamal2013年08月29日 19:31:15 +00:00Commented Aug 29, 2013 at 19:31
#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";
}
-
\$\begingroup\$ thanks :) but it's a bit complicated to me :D I've just started learning c++. \$\endgroup\$mitya221– mitya2212013年08月29日 21:22:32 +00:00Commented Aug 29, 2013 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– Jamal2013年08月29日 21:40:00 +00:00Commented Aug 29, 2013 at 21:40 -
\$\begingroup\$ well.. I understand it but I couldn't write such a program yet :) but thank you very much :) \$\endgroup\$mitya221– mitya2212013年08月30日 10:31:56 +00:00Commented Aug 30, 2013 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– Toby Speight2018年10月31日 09:53:56 +00:00Commented Oct 31, 2018 at 9:53
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;
Explore related questions
See similar questions with these tags.