Jeff King <peff@xxxxxxxx> writes:
> Yes, there are two ways to write this. With a conditional to initialize
> and return or to return the default, as we have here:
>
>> > >+ if (!git_config_get_bool("index.recordendofindexentries", &val))
>> > >+ return val;
>> > >+ return 0;
>
> Or initialize the default ahead of time, and rely on the function not to
> modify it when the entry is missing:
>
> int val = 0;
> git_config_get_bool("index.whatever", &val);
> return val;
>
> I think either is perfectly fine, but since I also had to look at it
> twice to make sure it was doing the right thing, I figured it is worth
> mentioning as a possible style/convention thing we may want to decide
> on.
I too think either is fine, and both rely on the git_config_get_*()
to modify the value return only when it sees that it is set.
I'd choose the latter when the default value is simple, as the
reader does not have to even know what the return value from the
git_config_get_*() function means to follow what is going on.
On the other hand, the former (i.e. the original by Jonathan) is
more flexible, and it makes it possible to write a piece of code,
which computes a default that is expensive to build only when
necessary, in the most natural way. The readers do need to be aware
of how the functin signals "I didn't get anything" with its return
value, though.
I do not mind standardizing on the latter, though. A caller with an
expensive default can initialize val to an impossible "sentinel"
value that signals the fact that git_config_get_*() did not get
anything, as long as the type has a natural sentinel (like -1 for a
bool to signal "unset"), and code that comes either immediately
after git_config_get_*() or much much later in the control flow can
check for the sentinel to see if it needs to compute the expensive
default.
