Constructive criticism/suggestions

This is a discussion on Constructive criticism/suggestions within the C Programming forums, part of the General Programming Boards category; I finally took the plunge, decided to register and join in on the discussion. I am using the book "Practical ...

  1. #1
    Registered User
    Join Date
    Sep 2006
    Posts
    8

    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.

  2. #2
    int x = *((int *) NULL); Cactus_Hugger's Avatar
    Join Date
    Jul 2003
    Location
    Banks of the River Styx
    Posts
    902
    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.
    long time; /* know C? */
    Unprecedented performance: Nothing ever ran this slow before.
    Any sufficiently advanced bug is indistinguishable from a feature.
    Real Programmers confuse Halloween and Christmas, because dec 25 == oct 31.
    The best way to accelerate an IBM is at 9.8 m/s/s.
    recursion (re - cur' - zhun) n. 1. (see recursion)

  3. #3
    Registered User
    Join Date
    Sep 2006
    Posts
    8
    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.

  4. #4
    Registered User SKeane's Avatar
    Join Date
    Sep 2006
    Location
    England
    Posts
    234
    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.

  5. #5
    Registered User
    Join Date
    Sep 2006
    Posts
    7

    Wink Easy to Read Chopped Down Version!

    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!

  6. #6
    Just Lurking Dave_Sinkula's Avatar
    Join Date
    Oct 2002
    Posts
    5,006
    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}?
    7. It is easier to write an incorrect program than understand a correct one.
    40. There are two ways to write error-free programs; only the third one works.*

  7. #7
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,506
    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
    }
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  8. #8
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Make them static. Or global. Then they'll automatically be initialized to zero.
    Hey, they just asked how, not what was a good idea.

    Quzah.
    Hope is the first step on the road to disappointment.

  9. #9
    Registered User
    Join Date
    Sep 2006
    Posts
    8
    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

    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. 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);
    }

  10. #10
    Registered User SKeane's Avatar
    Join Date
    Sep 2006
    Location
    England
    Posts
    234
    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).
    Last edited by SKeane; 09-29-2006 at 02:32 AM.

  11. #11
    Just Lurking Dave_Sinkula's Avatar
    Join Date
    Oct 2002
    Posts
    5,006
    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.
    7. It is easier to write an incorrect program than understand a correct one.
    40. There are two ways to write error-free programs; only the third one works.*

  12. #12
    Registered User
    Join Date
    Sep 2006
    Posts
    8
    Once I learn structures I will re-write it to use difftime but right now that just looks like gibberish to me.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Looking for constructive criticism
    By wd_kendrick in forum C Programming
    Replies: 16
    Last Post: 05-28-2008, 09:42 AM
  2. Constructive criticism, suggestions etc
    By BobS0327 in forum C Programming
    Replies: 3
    Last Post: 01-08-2006, 08:35 AM
  3. Constructive criticism highly appreciated!
    By muggizuggi in forum C++ Programming
    Replies: 17
    Last Post: 08-01-2005, 11:54 AM
  4. Constructive Feed Back (Java Program)
    By xddxogm3 in forum Tech Board
    Replies: 12
    Last Post: 10-10-2004, 03:41 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21