Dan Zwell [off-list ref] writes:
Previously, the Git->repository()->config('non-existent.key')
evaluated to as true in a vector context. Return an empty list
instead.
---
I don't know whether this breaks anything, because I don't use most of
the git perl scripts. I can't imagine that there is a script that
relies on the fact that config('non-existent.key') actually returns
(''), in an array context. Is this a reasonable change?
I did not examine the callers but my gut feeling is that it
would be simpler and cleaner to always return () without
checking the context. In scalar context:
sub null {
...
return ();
}
my $scalar = null();
would assign undef to $scalar anyway.
I generally try to stay away from functions that changes their
return values depending on the context, because they tend to
make reading the callers to find bugs more difficult. An
exception is a function that tries to mimic a built-in operator,
because reading the callers to such a function, as long as it is
clear which built-in the function is imitating, can apply the
same knowledge on how the callee would behave you already have
by knowing Perl itself.
The same thing can be said about functions with prototypes to
force certain context on the caller's side. Avoid it unless
there is a good reason.
Maybe it is just me, but that's my reaction.
Junio C Hamano wrote:
I did not examine the callers but my gut feeling is that it
would be simpler and cleaner to always return () without
checking the context. In scalar context:
sub null {
...
return ();
}
my $scalar = null();
would assign undef to $scalar anyway.
I generally try to stay away from functions that changes their
return values depending on the context, because they tend to
make reading the callers to find bugs more difficult.
<snip>
That's reasonable. I'll resend this as part of the git-add--interactive
color patches. This can be cherry-picked out, but some of the other
stuff I want to do depends on it (a helper function that I wrote,
config_with_default($repo, $key, $default)).
Dan
Hi,
On Thu, Nov 15, 2007 at 10:39:26PM -0800, Junio C Hamano wrote:
Dan Zwell [off-list ref] writes:
quoted
Previously, the Git->repository()->config('non-existent.key')
evaluated to as true in a vector context. Return an empty list
instead.
---
I don't know whether this breaks anything, because I don't use most of
the git perl scripts. I can't imagine that there is a script that
relies on the fact that config('non-existent.key') actually returns
(''), in an array context. Is this a reasonable change?
I did not examine the callers but my gut feeling is that it
would be simpler and cleaner to always return () without
checking the context.
[...]
I generally try to stay away from functions that changes their
return values depending on the context, because they tend to
make reading the callers to find bugs more difficult.
In fact, if you simply return without any value, it will evaluate to "false"
no matter which context it has been called in ("()" in list context, "undef"
in scalar context, etc.). So, it's generally a good idea to use "return;" to
indicate an error.
Cheers,
Sebastian
--
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/
Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin