I actually check all the participants and still can't find where it is missing the return value. Thanks for who look through my codes <3
bool Time::check()
{
if ((month < 1) || (month > 12)) return false;
switch (month)
{
case 1:case 3:case 5:case 7:case 8:case 10:case 12:
lastday = 31;
if ((day <= lastday) && (day > 0)) return true;
else return false;
break;
case 4:case 6:case 9:case 11:
lastday = 30;
if ((day <= lastday) && (day > 0)) return true;
else return false;
break;
case 2:
if ((year % 4) == 0)
{
lastday = 29;
if ((day <= lastday) && (day > 0)) return true;
else return false;
}
else
{
lastday = 28;
if ((day <= lastday) && (day > 0)) return true;
else return false;
}
}
}
CodePudding user response:
The big problem: Your compiler isn't smart enough. Sure, logically, it's impossible for the function as written not to return, but the compiler isn't tying together the preliminary range check with the switch
to realize any code that reaches the switch
will always match one of the cases.
The simplest solution is to just get rid of the manual range check and have it handled by the switch
's default
case, e.g.:
bool Time::check()
{
switch (month)
{
// Original cases here
default: // added
return false; // added
}
}
The code could be improved further by avoiding code duplication; the only thing differentiating your switch
case
s is lastvalue
. So limit them to that, and put the common code outside the switch
:
bool Time::check()
{
int lastday; // Declared outside the switch so it's available for final test
switch (month)
{
case 1:case 3:case 5:case 7:case 8:case 10:case 12:
lastday = 31;
break;
case 4:case 6:case 9:case 11:
lastday = 30;
break;
case 2:
lastday = year % 4 == 0 ? 29 : 28; // Note: Leap year rules are more complicated than this; look 'em up
break;
default:
return false; // Not a valid month; could do lastday = 0; break; to stick to a single return but that's liable to confuse
}
return day <= lastday && day > 0;
}
CodePudding user response:
The compiler tries to understand your code, but not always it can understand everything. A different code with similar issue is:
int foo(int x){
if (x <= 1 || x >= 3) return false;
if (x == 2) return true;
}
Either x
is 2
or x
is not 2
, then it is either <= 1
or >= 3
. Hence the function always returns. Nevertheless, the compiler issues a warning:
<source>:6:1: error: control reaches end of non-void function [-Werror=return-type]
Consider this obfuscated version which is maybe similar to what the compiler uses for that anlysis:
int foo(int x){
if (some_condition) return false;
if (some_other_condition) return true;
}
If neither some_condition
nor some_other_condition
is true
the function does not return and that is undefined behavior.
Long story short, The compiler is clever, but not clever enough to see that you are covering all possible values for month
. To fix it remove the initial if
and add a default
case instead. Or rather move everything common to all cases outside of the switch:
bool Time::check()
{
int lastday = 0;
switch (month)
{
case 1:case 3:case 5:case 7:case 8:case 10:case 12:
lastday = 31;
case 4:case 6:case 9:case 11:
lastday = 30;
break;
case 2:
if ((year % 4) == 0)
{
lastday = 29;
}
else
{
lastday = 28;
}
default: lastday = -1;
}
return ((day <= lastday) && (day > 0));
}
I would suggest to move the logic to find days per month out of the function check
. And you can use an array:
int days(int month,int year)
{
int days_per_month[] = {31,28,31,30,31,30,31,31,30,30,31,30,31};
return days_per_month[month-1] (month==2 && year%4==0);
}