Re: [PATCH 2/2] Git/Packet.pm: use 'if' instead of 'unless'

[Jonathan Nieder]

Hi,

Christian Couder wrote:

> The code is more understandable with 'if' instead of 'unless'.
>
> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
> ---
>  perl/Git/Packet.pm | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

I'm agnostic about that.  In some ways it seems like an odd change,
since it is changing from

        if (exceptional case) {
                die "explanation";
        }
        common case;

to

        if (!exceptional case) {
                common case;
        }
        die "explanation";

when we usually prefer the former (getting the exceptional case over
with early so the reader can concentrate on the common case).  In
perl, it might be even more idiomatic to do

        die "explanation" if exceptional case;
        common case;

but Documentation/CodingGuidelines appears to recommend not going that
far. :)

[...]
> --- a/perl/Git/Packet.pm
> +++ b/perl/Git/Packet.pm
> @@ -68,16 +68,16 @@ sub packet_bin_read {
>  
>  sub remove_final_lf_or_die {
>       my $buf = shift;
> -     unless ( $buf =~ s/\n$// ) {

For readability, I find this whitespace within parens more distracting.

Documentation/CodingGuidelines covers that for C programs but not for
Perl.  Do we have a preferred style either way about it?

Thanks,
Jonathan