Did I over complicate this code?

[cdea06]

So I just started a "C" class and I've done my first few assignments but I feel like I'm over complicating the codes by using to many if else statements. Here's the assignment and my code. Please let me know what you think:

Write a program that asks the user to enter two dates and then indicates which date comes earlier on the calendar.

Your output should match the example below (except that the input will be different for other executions!) User input is underlined.

Enter first date (mm/dd/yyyy): 3/6/2008
Enter the second date (mm/dd/yyyy): 5/17/2007
5/17/2007 is earlier than 3/6/2008

/*
  Program assignment name: 2_Dates

  Author:Christopher D*****
*/

#include <stdio.h>

int main (void)
{
   int  mm, dd, yyyy, mm2, dd2, yyyy2;
   printf("Enter first date (mm/dd/yyyy): ");
   scanf("%d/%d/%d", &mm, &dd, &yyyy);
   printf("Enter second date (mm/dd/yyyy): ");
   scanf("%d/%d/%d", &mm2, &dd2, &yyyy2);
   
   if (yyyy < yyyy2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
   }
   else if (yyyy > yyyy2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
   }
   else if (yyyy == yyyy2 && mm < mm2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
   }
   else if (yyyy == yyyy2 && mm > mm2)
   {    
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
   }
   else if (dd < dd2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
   }
   else if (dd2 > dd)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
   }
   else
   {
      printf("The dates you entered are the same\n");
   }
   return 0; 
}

[dev90]

1)

Yes, it is complicated.
you should use flag and store 1 if 1st date is smaller.and use break; in if block;
and use nested block istead of comparing year each time.
And print sataement will come at last according to falg value

2) The easy way is count the number of days and decide which is smaller/greater.

[subith86]

I don't see any validity checks done for your dates. He can enter invalid dates like 33/33/2012.
Once the dates are validated, the job is easy.

day1 = yyyy1 * 10000 + mm1 * 100 + dd1
day2 = yyyy2 * 10000 + mm2 * 100 + dd2

Check which is greater out of day1 and day2. So simple, isn't it :P

But there is another potential error in your program. Your variables mm1, dd1 are declared as integers and in your console out put you ask the user to enter in mm/dd/yyyy format. So for September, the user will enter "09". But since mm1 is declared as int, "09" will be treated as octet number and you'll get run time errors, since 8 & 9 are not allowed in octet system. So better use strings.

[WaltP]

you should use flag and store 1 if 1st date is smaller.and use break; in if block;

Have you ever used a break in an if() block? It doesn't work.

I don't see any validity checks done for your dates. He can enter invalid dates like 33/33/2012.

I would assume validity checks in this program are moot. It's just to figure out how to compare dates in general

day1 = yyyy1 * 10000 + mm1 * 100 + dd1
day2 = yyyy2 * 10000 + mm2 * 100 + dd2

Check which is greater out of day1 and day2. So simple, isn't it :P

Nice. Simple and workable, even with invalid dates. I'd use some parentheses though. Makes the equation easier to read.

But there is another potential error in your program. Your variables mm1, dd1 are declared as integers and in your console out put you ask the user to enter in mm/dd/yyyy format. So for September, the user will enter "09". But since mm1 is declared as int, "09" will be treated as octet number and you'll get run time errors, since 8 & 9 are not allowed in octet system. So better use strings.

Completely untrue. Write a test program and see.

[subith86]

Completely untrue. Write a test program and see.

Thanks WaltP. I didn't know that. I posted it with the knowledge that int i = 09; gives an error.
So, I believe scanf will be doing some formatting to 09. Am I right?

[WaltP]

Thanks WaltP. I didn't know that. I posted it with the knowledge that int i = 09; gives an error.
So, I believe scanf will be doing some formatting to 09. Am I right?

No, scanf() reads in what you type into %d (decimal). 09 is valid decimal. It's only the internal use that a beginning 0 indicates octal.

On the other hand, you are correct if you use %o in scanf() .

[cdea06]

Thanks guys, I took your suggestions and rewrote my code. I tried something like that at fist but wasn't using high enough multiples. I still have no validity check, which bugs me, but I don't think it matters on this assignment since it's only week 2 of "C" for beginners and we haven't learned anything yet that will be useful. I thought about using a while loop to check it but there seems to be too many variables for that to work. ie. while (mm <= 12 && dd <=31)....
For my own curiosity (I wont be submitting it with my code since it's outside of this simple assignment) how would you change my code to accomplish this? Here's what I have now:

/*
  Program assignment name: 2_Dates

  Author:Christopher D*****
*/

#include <stdio.h>

int main (void)
{ 
   int mm, dd, yyyy, mm2, dd2, yyyy2, first_date = 0, second_date = 0; 
   
   printf("Enter first date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm, &dd, &yyyy); 
   printf("Enter second date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm2, &dd2, &yyyy2); 

   first_date = (yyyy * 10000) + (mm * 100) + dd; 
   second_date = (yyyy2 * 10000) + (mm2 * 100) + dd2;
   
   if (first_date < second_date) 
   { 
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
   }
   else if (first_date > second_date)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
   }
   else if (first_date == second_date)
   {
      printf("The dates you entered are the same\n");
   }
   else
   {
      printf("You entered an invalid date format!");
   }

   return 0;
}

[subith86]

Oh, no!!!
No while loop to check for validity. But since you are a beginner, I understand you'll take some time to get a hang of things.

Validations are like checking whether a condition is met or not. And for a condition, the right candidate is if(condition).

Something like this

if((mm > 0) && (mm <= 12) && (dd > 0) && (dd <= 31) && (yyyy >= 0))
{
    //valid
}
else
{
    //invalid
}

But there is a problem here. If a user enter Feb-31, it gets validated. So the validation thing is a bit complex for a 2-week guy. You have to use a very complicated if-condition to achieve the purpose. Or, I would go with arrays. Storing the number of days in each month in an array and validating it. But I don't think so that you are taught arrays by this time. So, leave the validation part as of now.

Now coming to your existing code, i feel the else part is not needed. The three possibilities between two integers are '<', '>' and '==' all of which are handled in separate blocks. Whatever combination of integers you give, it will never reach the "else" part.

And glad to see that you are using int main(void) as the prototype of your main function, unlike other beginners (including me, when I was a beginner) who use void main() :) Also mark the thread as solved, when done :)

[rubberman]

The rule of thumb is... EVERYONE over complicates their C code! Applying the KISS principal to code, especially C and C++ code is critical in building reliable and understandable programs/systems. Anyway, I will look at your code and comment again in another message.

[rubberman]

Ok. Some comments first (remembering the KISS principal):

1. Initialize ALL variables to sane values (if only 0 or NULL) when you declare them.
2. Be orthogonal in your branches. IE, keep things that are related to yyyy/yyyy2 in the yyyy/yyyy2 branches, the things that are related to mm/mm2 in theirs, and the things related to dd/dd2 in theirs. I'll show what I mean below.
3. Validate ALL input. IE, make sure that the input for month, day, and year are sane, in that the month MUST be >= 1 and <= 12, and use an array for the number of days in a month to validate that. The only glitch here is dealing with leap years, which you do not deal with. As for years, have a valid range of them, and DO check for leap years. There are good algorithms for computing them - including in Wikipedia, so I won't elaborate here. Consider it a study assignment! :-)

Ok, here is what I think would be better:

/*
  Program assignment name: 2_Dates

  Author:Christopher D*****
*/

#include <stdio.h>

int main (void)
{ 
   int mm = 0, dd = 0, yyyy = 0, mm2 = 0, dd2 = 0, yyyy2 = 0;
   
   printf("Enter first date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm, &dd, &yyyy); 

/* Validate mm, dd, yyyy here. */

   printf("Enter second date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm2, &dd2, &yyyy2); 

/* Validate mm2, dd2, yyyy2 here. */

   if (yyyy < yyyy2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
   }
   else if (yyyy > yyyy2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
   }
   else
   {
      if (mm < mm2)
      {
         printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
      }
      else if (mm > mm2)
      {    
         printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
      }
      else
      {
         if (dd < dd2)
         {
            printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
         }
         else if (dd2 > dd)
         {
            printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
         }
         else
         {
            printf("The dates you entered are the same\n");
         }
      }
   }
   return 0;
}

The purpose of nesting after the simple "else" statements is to make clear the distinctions between year, month, and day deltas.

Now, to make all of this REALLY simple, take the year, month, and day. Convert them into Julian dates, and then all you need is a simple 3 line if/else block. IE,

/*
  Program assignment name: 2_Dates

  Author:Christopher D*****
*/

#include <stdio.h>

int main (void)
{ 
   unsigned int mm = 0, dd = 0, yyyy = 0, mm2 = 0, dd2 = 0, yyyy2 = 0;
   unsigned int j1 = 0, j2 = 0;
   
   printf("Enter first date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm, &dd, &yyyy); 

/* Validate mm, dd, yyyy here. */

   printf("Enter second date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm2, &dd2, &yyyy2); 

/* Validate mm2, dd2, yyyy2 here. */

/* Convert dates to Julian - the code to do that can be considered an exercise. */
   j1 = toJulian(yyyy, mm, dd);
   j2 = toJulian(yyyy2, mm2, dd2);

   if (j1 < j2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm, dd, yyyy, mm2, dd2, yyyy2);
   }
   else if (j1 > j2)
   {
      printf("%2d/%2d/%4d is earlier than %2d/%2d/%4d\n", mm2, dd2, yyyy2, mm, dd, yyyy);
   }
   else
   {
      printf("The dates you entered are the same\n");
   }
   return 0;
}

Again, the algorithm to convert dates (and optionally time) to Julian values can be found in the Wikipedia.

[WaltP]

1. Initialize ALL variables to sane values (if only 0 or NULL) when you declare them.

Unnecessary. Only values that are not read in or set in the code need to be initialized. In your example

int mm = 0, dd = 0, yyyy = 0, mm2 = 0, dd2 = 0, yyyy2 = 0;
   
   printf("Enter first date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm, &dd, &yyyy);

initializing mm,dd,yyyy are completely unnecessary and a waste of time.

[rubberman]

Unnecessary. Only values that are not read in or set in the code need to be initialized. In your example

int mm = 0, dd = 0, yyyy = 0, mm2 = 0, dd2 = 0, yyyy2 = 0;
   
   printf("Enter first date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm, &dd, &yyyy);

initializing mm,dd,yyyy are completely unnecessary and a waste of time.

Speaking from experience, your comment is correct on the face of it, but all software changes over time, and this has been the root cause of MANY major system failures over the years! My personal process says "never create a variable that is not initialized". I have written quite literally millions of lines of code over the past 30+ years that runs some of the biggest manufacturing corporations, US Navy refit centers, stealth fighter avionics manufacturing plants, 80% of the semiconductor manufacturing plants in the world, and this RULE has helped provide the highest quality software in 365x24 environments anywhere. FWIW, I am currently Senior Systems Engineer for the largest cell phone manufacturer in the world...

[rubberman]

Unnecessary. Only values that are not read in or set in the code need to be initialized. In your example

int mm = 0, dd = 0, yyyy = 0, mm2 = 0, dd2 = 0, yyyy2 = 0;
   
   printf("Enter first date (mm/dd/yyyy): "); 
   scanf("%d /%d /%d", &mm, &dd, &yyyy);

initializing mm,dd,yyyy are completely unnecessary and a waste of time.

One last comment about yours: what happens when the user hits the RETURN key or Ctrl-D after just inputting the month and day? Is the year still sane?

[WaltP]

One last comment about yours: what happens when the user hits the RETURN key or Ctrl-D after just inputting the month and day? Is the year still sane?

Yes. Because, since you are discussing best practices, when you test the return code from scanf() you will find all values were not entered and handle the improper input. You do advocate always testing the return values from all functions, don't you? :icon_wink:

[rubberman]

Yes. Because, since you are discussing best practices, when you test the return code from scanf() you will find all values were not entered and handle the improper input. You do advocate always testing the return values from all functions, don't you? :icon_wink:

Absofragginglutely!

[rubberman]

FWIW, the software I have been writing for over 20 years has had to be better than 6-sigma in reliability, especially since an hour of downtime for our customers costs them in excess of $10M USD. We program for failure, and anticipate failure modes. We extensively use tools like Purify, Quantify, Pure Coverage, and other such in conjunction with extensive unit and system testing, so that when software is delivered, we have some reasonable expectation that it will work as designed.

[cdea06]

You know I think I've learned more from this post than I have in the class so far! I major in Applied Physics and minor in Computer Programing with my eventual grad school(2014) being Computational Physics so as you can image I'm pretty excited about learning "C". Thanks again for all your help. This is why I use this forum!

[rubberman]

I know this thread was last posted to a year ago, but just a comment to cdea06 - my wife is a PhD paricle physicist who mostly does computational physics at Fermi National Laboratory here in Illinois! I wish you the best in your chosen career! :-)