Re: [PATCH][GIT.PM 2/3] Getting rid of throwing Error::Simple objects in favour of simple Perl scalars which can be caught in eval{} blocks

From: Andrew Sayers <andrew-git@pileofstuff.org>
Date: May 19, 2012, 09:38 UTC
Message-ID: <4FB76A21.7000801@pileofstuff.org>
In-Reply-To: <1337411317-14931-2-git-send-email-subs.zero@gmail.com>

I'll limit myself to a style review here - other people can say better than me about the deeper issues.

On 19/05/12 08:08, Subho Sankar Banerjee wrote: <snip>

Show 6 quoted lines

> @@ -160,7 +160,7 @@ sub repository {
>  	if (defined $args[0]) {
>  		if ($#args % 2 != 1) {
>  			# Not a hash.
> -			$#args == 0 or throw Error::Simple("bad usage");
> +			$#args == 0 or die "bad usage";

This is valid and no worse than before, but I find this use of the "or" operator slightly confusing. I find it easier to read either:

	<verb> or <fail>
	OR:
	<fail> unless <noun>

For example:

	do_something($foo) or die "couldn't do_something with '$foo'";
	OR:
	die "'$foo' is not a something" unless is_something($foo);

<snip>

Show 6 quoted lines

> @@ -1041,7 +1041,7 @@ sub _temp_cache {
>  
>  		($$temp_fd, $fname) = File::Temp->tempfile(
>  			'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
> -			) or throw Error::Simple("couldn't open new temp file");
> +			) or die "couldn't open new temp file";

This is a good example of where I think "or" is appropriate.

Think of it in terms of an English sentence. Which of these would you find easier to read:

	It is raining or go out and play
	OR:
	Go out and play unless it is raining

	Find your umbrella or cancel the trip
	OR:
	Cancel the trip unless find your umbrella

A bit of background for people who aren't (primarily) Perl programmers:

As an expressive language that promotes "more than one way to do it", Perl has a long tradition of supporting many redundant ways of spelling "if (...) { ... }". Common examples include:

	if ( $x ) { do_something() }
	do_something() if $x;

	unless ( $x ) { do_something() }
	do_something() unless $x;

	$x && do_something();
	$x || do_something();

	$x and do_something();
	$x or do_something();

Sometimes people find very practical reasons why these aren't good programming practice, but the rest of the time everyone just argues about whether they're good grammar.

The "&&" and "||" operators are an example of bad programming practice - these operators have relatively high precedence, so tend to behave unintuitively when used in (often regrettably) complex ways. The "and" and "or" operators behave just like "&&" and "||", but with a precedence low enough to avoid weirdness. See [1] for an example.

Some people consider anything but a traditional prefix-if() statement to be bad grammar (I believe "Perl Best Practices" makes the argument, which is definitive for many people). Other people say anything in the language is by definition fair game. The rest of us spend a lot of time making arguments like the above, and frankly I think we gain more from the debate than the conclusion.

	- Andrew

[1] http://perldoc.perl.org/perlop.html#C-style-Logical-Defined-Or