Compilers
Welcome to Code Review. First of all, "better" is a very personal opinion term, unless it has been defined. From a compiler's point of view, both functions do the same. As compilers are data flow analysis beasts, a decent one will simplify the function's flow to the following variant (see https://godbolt.org/z/yZCPOv):
ret = 0
if year & 3 /= 0
goto end
ret = 1
if year % 100 /= 0
goto end
ret = (year % 400 == 0)
end:
return ret
return 0
As you can see, with -Os
(or any other optimization level), your functions will result in the same assembler. This already answers your first (implicit) question:
I was wondering whether it would be correct to change this function by using guard clauses like below
"Better"
Since both generate the same assembler, both are the same. We can of course check that by hand, as there are only four equivalence classes we have to check, but that's left as an exercise.
Regardless, the better way is the one that's easier to understand and follows fits into the rest of your code. If all your code is written in the first style, use that one. If you usually bail out early, use the latter. Neither have advantages over the other as soon as we enable optimizations. Without optimizations, your latter variant uses less jumps (but only a single one), and that's still compiler dependent.
In the end, use the variant you like most, but make sure you can still understand and explain it to yourself or someone else at a later time.
Just for completion, I personally prefer variants that have less jumps but are still readable, so I'd probably write something along
//! @returns whether @c year is a leap year
bool IsLeapYear(int year)
{
//! A year is a leap year if its divisible by 4, but not by 100,
//! @b unless it's also divisible by 400.
return (year % 4 == 0)
&& ((year % 100 != 0) || (year % 400 == 0));
}