Constructive criticism/suggestions

I finally took the plunge, decided to register and join in on the discussion. :)

I am using the book "Practical C Programming" to help teach myself how to program in C. I've gone through the beginning chapters and done one of the exercises. I was hoping I could get suggestions on how to improve on it or ways I could do things more efficiently. The exercise reads as follows:

Write a program to perform date arithmetic such as how many days there are between 6/6/90 and 4/3/92.

This is what I have so far:

Code:

---

#include <stdio.h>

int dayspassed(int month, int day, int year, int sMonth, int sDay, int sYear);

int main()
{
    char buffer[20] = {0};
    int day = 0, month = 0, year = 0;
    int sDay = 0, sMonth = 0, sYear = 0;

    printf("\nInput first date in MM/DD/YY format: ");
    fgets(buffer, sizeof(buffer), stdin);
    sscanf(buffer, "%d/%d/%d", &month, &day, &year);

    if ((month > 12) || (day > 31))
        printf("Error: Invalid date.\n");
    else
        printf("You chose %d/%d/%d.\n", month, day, year);

    printf("\nInput second date in MM/DD/YY format: ");
    fgets(buffer, sizeof(buffer), stdin);
    sscanf(buffer, "%d/%d/%d", &sMonth, &sDay, &sYear);

    if ((sMonth > 12) || (sDay > 31))
        printf("Error: Invalid date.\n");
    else
        printf("You chose %d/%d/%d.\n", sMonth, sDay, sYear);

    printf("The number of days between %d/%d/%d and %d/%d/%d is %d.",
    month, day, year, sMonth, sDay, sYear, dayspassed(month, day, year, sMonth, sDay, sYear));

    return (0);
}

int dayspassed(int month, int day, int year, int sMonth, int sDay, int sYear)
{
    int diffDay = 0, diffMonth = 0, diffYear = 0, days = 0;

    diffDay = sDay - day;
    diffMonth = sMonth - month;
    diffYear = sYear - year;

    days = diffDay + (diffMonth * 31) + (diffYear * 365);

    return (days);
}

---

I'm mainly looking for information on some methods of input validation, but suggestions on features to add would be great also.

I also was wondering if there is any way to initialize all the variables in one line to 0. Take this piece of code for example:

Code:

---

int day = 0, month = 0, year = 0;
int sDay = 0, sMonth = 0, sYear = 0;

---

I figured you could just do day = month = year = 0 but that doesn't seem to work.

Thanks in advance and flame away if I did something stupid. :cool:

Welcome to boards.

You can do day = month = year = 0, but not during the declaration. Examples:

Code:

---

int day = 0, month = 0, year = 0, sDay = 0, sMonth = 0, sYear = 0;
_or_
int day, month, year, sDay, sMonth, sYear;
day = month = year = sDay = sMonth = sYear = 0;

---

Personally, I think the two lines you're using is the best way. It separates out two groups of variables and initializes them.

Also, for dayspassed() - if only things were that simple... hopefully you haven't forgotten about leapyears / months having less than 31 days. Taking that into account makes the problem much more complex, so I can understand if you left it out. (But it does say there are 2 days between 6/30/1 and 7/1/1) Also, issues arise if dates cross 1/1/0. Good ole Y2K. (Four digit years would help here) (It states +/-36506 days between 12/31/99 and 1/1/00) And the order of the dates matters.

Quote:

---

Originally Posted by Cactus_Hugger

Welcome to boards.

You can do day = month = year = 0, but not during the declaration. Examples:

Code:

---

int day = 0, month = 0, year = 0, sDay = 0, sMonth = 0, sYear = 0;
_or_
int day, month, year, sDay, sMonth, sYear;
day = month = year = sDay = sMonth = sYear = 0;

---

Personally, I think the two lines you're using is the best way. It separates out two groups of variables and initializes them.

Also, for dayspassed() - if only things were that simple... hopefully you haven't forgotten about leapyears / months having less than 31 days. Taking that into account makes the problem much more complex, so I can understand if you left it out. (But it does say there are 2 days between 6/30/1 and 7/1/1) Also, issues arise if dates cross 1/1/0. Good ole Y2K. (Four digit years would help here) (It states +/-36506 days between 12/31/99 and 1/1/00) And the order of the dates matters.

---

I left the leap years/months with less days because it was too complicated like you said. I am in the process of coming up with some solutions though. The four digit years thing was something I didn't even notice.

Only thing I'm not quite sure on is how to make the date input order not matter.

Thanks for the feedback. :cool:

Maybe you should look at the difftime() function. This will tell you the difference (in seconds) between two times since the epoch.

If you convert your dates into time since the epoch (time_t's) then the function will automatically take into account leap years etc. It doesn't cope with leap seconds, but nothing's that perfect.

If you know the difference in seconds between 2 dates, and you know how many seconds there are in a day (24 x 60 x 60) then you can work out the days.

If you look at the time.h functions you'll see how to construct a "struct tm" and convert it to a time_t.

Quote:

---

I also was wondering if there is any way to initialize all the variables in one line to 0. Take this piece of code for example:

---

Code:

---

#include <stdio.h>

int main()
{
    int month=0, day=0, year=0, sMonth=0, sDay=0, sYear=0, dMonth=0, dDay=0, dYear=0, days=0;
    printf ("Input first date in MM/DD/YY format: \n\n");
    scanf ("%d/%d/%d", &month, &day, &year);

    if ((month > 12) || (day > 31))
        printf("Error: Invalid date.\n");
    else
        printf("You chose %d/%d/%d.\n\n", month, day, year);

    printf("Input second date in MM/DD/YY format: \n\n");
    scanf("%d/%d/%d", &sMonth, &sDay, &sYear);

    if ((sMonth > 12) || (sDay > 31))
        printf("Error: Invalid date.\n");
    else
        printf("You chose %d/%d/%d\n\n", sMonth, sDay, sYear);

    dDay = sDay - day;
    dMonth = sMonth - month;
    dYear = sYear - year;
    days = dDay + (dMonth * 31) + (dYear * 365);

    printf("The number of days between %d/%d/%d and %d/%d/%d is %d.\n",  month, day, year, sMonth, sDay, sYear, days);

    return (0);
}

---

~EvoTone~
__________________________________________________ ____________

If you wake up every morning and can think of nothing else but, programming you are a programmer!

Quote:

---

Originally Posted by LineOFire

I also was wondering if there is any way to initialize all the variables in one line to 0.

---

Put 'em in a struct and init the struct to {0}?

Or two instances of a struct

Code:

---

struct date { int y, m, d; };

---

Then perhaps

Code:

---

int main ( ) {
  struct date start, end;
  readDate( &start );
  readDate( &end );
}

---

And a function to read and validate

Code:

---

void readDate ( struct date *d ) {
  scanf ("%d/%d/%d", &d->m, &d->d, &d->y);
  // validate
}

---

Make them static. Or global. Then they'll automatically be initialized to zero. ;)
Hey, they just asked how, not what was a good idea. :D

Quzah.

Most of the things you guys suggested I haven't learned/have no idea what you're talking about yet. Only finished learning the loops and just starting pointers:p

I made lots of changes to the code this time around. Separated it into more functions, added calendar realism (April, June, September, November only have 30 days, support for leap years). I'll look into the structures and other suggestions you guys mentioned.

Right now it's a mess of if/else statements that took me a while to logic through. Something tells me there's an easier way to do this. :D Let me know if you find any bugs or any other suggestions you might have.

Code:

---

#include <stdio.h>
#define ERROR 1
#define SUCCESS 0

int ioDates(void);
int valiDate(int month, int day, int year);
int isLeapYear(int year);
int daysPassed(int month, int day, int year, int sMonth, int sDay, int sYear);

char buffer[20] = {0};
int month = 0, day = 0, year = 0;
int sMonth = 0, sDay = 0, sYear = 0;

int main()
{
    ioDates();

    return (0);
}

int ioDates(void) /* Gets the two dates from the user */
{
    printf("\nInput first date in MM/DD/YYYY format: ");
    fgets(buffer, sizeof(buffer), stdin);
    sscanf(buffer, "%d/%d/%d", &month, &day, &year);

    if ((valiDate(month, day, year)) == SUCCESS)
    {
        printf("\nInput second date in MM/DD/YYYY format: ");
        fgets(buffer, sizeof(buffer), stdin);
        sscanf(buffer, "%d/%d/%d", &sMonth, &sDay, &sYear);
    }
    else
        ioDates();

    if ((valiDate(sMonth, sDay, sYear)) == SUCCESS)
    {
        printf("\nThe number of days between %d/%d/%d and %d/%d/%d is %d.",
        month, day, year, sMonth, sDay, sYear, daysPassed(month, day, year, sMonth, sDay, sYear));
    }
    else
        ioDates();

    return (0);
}

int valiDate(int month, int day, int year) /* Makes sure all dates are valid */
{
    if (month > 0 && month < 13 && day > 0 && day < 32 && year > 0)
    {
        if (month == 2)
        {
            if(((isLeapYear(year)) == 0) && day > 29)
            {
                printf("\nERROR: There are only 29 days of February in %d.\n", year);
                return (ERROR);
            }
            else if (((isLeapYear(year)) == 1) && day > 28)
            {
                printf("\nERROR: There are only 28 days of February in %d.\n", year);
                return (ERROR);
            }
        }
        else if ((month == 4) && (day > 30))
        {
            printf("\nERROR: There are only 30 days in April.\n");
            return (ERROR);
        }
        else if ((month == 6) && (day > 30))
        {
            printf("\nERROR: There are only 30 days in June.\n");
            return (ERROR);
        }
        else if ((month == 9) && (day > 30))
        {
            printf("\nERROR: There are only 30 days in September.\n");
            return (ERROR);
        }
        else if ((month == 11) && (day > 30))
        {
            printf("\nERROR: There are only 30 days in November.\n");
            return (ERROR);
        }
        else
        {
            printf("The date you typed is %d/%d/%d.\n", month, day, year);
            return (SUCCESS);
        }
    }
    else
    {
        printf("\nERROR: Invalid date. Please try again.\n");
        return (ERROR);
    }
}

int isLeapYear(int year) /* Determines if the year is a leap year */
{
    if (year % 4 == 0)
    {
        if (year % 100 == 0)
        {
            if (year % 400 == 0)
            {
                return (SUCCESS);
            }
            else
            {
                return (ERROR);
            }
        }
        else
        {
            return (SUCCESS);
        }
    }
    else
    {
        return (ERROR);
    }
}

int daysPassed(int month, int day, int year, int sMonth, int sDay, int sYear) /* Calculates the days between each date */
{
    int diffDay = 0, diffMonth = 0, diffYear = 0;
    int daysBetween = 0, daysInMonth = 31, daysInYear = 365;

    diffDay = sDay - day;
    diffMonth = sMonth - month;
    diffYear = sYear - year;

    if ((month || sMonth) == (4 || 6 || 9 || 11))
        daysInMonth = 30;

    if (month == 2)
    {
        if ((isLeapYear(year)) == SUCCESS)
        {
            daysInMonth = 29;
            daysInYear = 366;
        }
    }

    daysBetween = diffDay + (diffMonth * daysInMonth) + (diffYear * daysInYear);

    return (daysBetween);
}

---

Code:

---

int isLeapYear(int year) /* Determines if the year is a leap year */
{
    if ( ((year % 4) == 0) && ( ((year % 100) != 0) || ((year % 400) == 0) ) )
    {
        return( SUCCESS );
    }
   
    return( ERROR );
}

---

Try your existing program with the following dates ...

01/01/2000
12/31/1999

The answer should be 1 day (you might indicate +/- if you want).

If you take the difftime approach already suggested, things can become fairly simple. But date/time functions are, on the whole, not a lot of fun (as perhaps you are beginning to appreciate).

With all that isn't validating whether or not the date is valid set aside, it could be something like this.

Code:

---

/*
 * Write a program to perform date arithmetic such as how many days there are
 * between 6/6/90 and 4/3/92.
 */

#include <stdio.h>
#include <time.h>

int dayspassed(struct tm *earlier, struct tm *later)
{
  time_t early = mktime(earlier), late = mktime(later);
  if ( early != (time_t)-1 && late != (time_t)-1 )
  {
      return difftime(late, early) / (60 * 60 * 24);
  }
  return -1; /* pick a better value or a different method */
}

int main(void)
{
  struct tm one = {0}, two = {0};

  /*
    * I'll skip user input.
    */

  one.tm_year = 90;    /* years since 1900 */
  one.tm_mon  = 6 - 1; /* months are 0 - 11 */
  one.tm_mday = 6;

  two.tm_year = 92;    /* years since 1900 */
  two.tm_mon  = 4 - 1; /* months are 0 - 11 */
  two.tm_mday = 3;

  printf("dayspassed() = %d\n", dayspassed(&one,&two));

  return 0;
}

/* my output
dayspassed() = 667
*/

---

*I also leave you to sort out the m/d/y vs d/m/y stuff.

Once I learn structures I will re-write it to use difftime but right now that just looks like gibberish to me. :(