2
\$\begingroup\$

I'm supposed to make a program that will tell me the number of the day of a certain date. So I input the date, and it will tell me which number day it is. (i.e: 1.1.2014 would be day 1, and 30.4.2012 would be 121) The program in its current state works, but it looks a little sloppy (or I'm just too self-conscious). I'm somewhat new to programming and was wondering if I can improve it, or if I can make it less sloppy.

#include <stdio.h>
#include <stdlib.h>

valid(int day, int month, int year)
{
    int st[12]={31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
    int copy[12]={31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
    if(year % 4 != 0)
        {
        if(day < 1 ||  day > st[month-1] || month < 1 || month > 12 || year < 0)
            return 0;
        else
        return 1;
    }
    else
        if(day < 1 ||  day > copy[month-1] || month < 1 || month > 12 || year < 0)
            return 0;
        else
            return 1;

}

int main()
{
    int day, month, year, sum;
    scanf("%d", &day);
    scanf("%d", &month);
    scanf("%d", &year);
    printf("Date is %d.%d.%d", day, month, year);
    sum = day;
    int st[12]={31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
    int copy[12]={31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
    if(valid(day, month, year) == 1)
        {
        if(year % 4 == 0)
            for(int i = 0; i < month; i++)
                sum = sum + copy[i-1];
        else
            for(int i = 0; i < month; i++)
                sum = sum + st[i-1];
        printf("\nDay number is %d", sum);
        }
    else
        printf("\nInvalid date");

}

The function valid checks whether my date is valid, so that I cannot input a wrong date. I also check for leap years inside my main program. I have an array which has the number of days in every month, and another one for leap years.

\$\endgroup\$
2
  • \$\begingroup\$ You could improve it by having the correct leap year calculation, which is every 4th year except every 100th year except every 400th year. Your program accepts 29 2 1900 which was not a valid date. It also lacks a return value in the function valid() declaration. \$\endgroup\$
    – Weather Vane
    Oct 10, 2015 at 16:08
  • \$\begingroup\$ Instead of having an array with the number of days in each month, you could just have the cumulative number of days (0, 31, 59, 80...) No need to lopp then. \$\endgroup\$
    – Anders
    Oct 10, 2015 at 16:11

1 Answer 1

2
\$\begingroup\$

The compiler warns you

The gcc compiler doubles as a code analysis tool. Running on your code it found the following potential problems:

year.c:4:1: warning: return type defaults to ‘int’
 valid(int day, int month, int year)
 ^
year.c: In function ‘main’:
year.c:26:5: warning: ignoring return value of ‘scanf’, declared with attribute warn_unused_result [-Wunused-result]
     scanf("%d", &day);
     ^
year.c:27:5: warning: ignoring return value of ‘scanf’, declared with attribute warn_unused_result [-Wunused-result]
     scanf("%d", &month);
     ^
year.c:28:5: warning: ignoring return value of ‘scanf’, declared with attribute warn_unused_result [-Wunused-result]
     scanf("%d", &year);
     ^

The last 3 warnings are no big deal in a toy like this, but the first is important: you forgot saying that the functions returns an int, greatly confusing the future reader of your code as he may not know by heart that unspecified return type defaults to int.

The (not)-boolean return antipattern

    if(day < 1 ||  day > st[month-1] || month < 1 || month > 12 || year < 0)
        return 0;
    else
    return 1;

Means that if day < 1 || day > st[month-1] || month < 1 || month > 12 || year < 0 is true you return 0 (false) otherwise (if it is false) you return true.

  • expression_value = true -> return_value = false
  • expression_value = false -> return_value = true

Just return the negation: !

return ! (day < 1 ||  day > st[month-1] || month < 1 || month > 12 || year < 0);

The same in general applies when there is no negation, booleans shall be returned direclty, not after an if - else

Use a negative check to reduce nesting

If a pre-condition is not met, just abort early:

if(! valid(day, month, year)) {
    puts("\nInvalid date\n");
    return 1; // non-zero return code signals error.
}

puts

printf has a lot of power, when dealing with simple strings puts is preferred (see the example above).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.