Re: [PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message

[Junio C Hamano <gitster@pobox.com>]

Michael Witten <mfwitten@gmail.com> writes:

> +				die "Server does not support STARTTLS: " . $smtp->message . "\n"
> +					unless $smtp->code == 220;

Statement modifiers merely make things even less readable, especially when
the conditional is the unlikely case.  Please do not add more of them.

	do this;
	do that;
	do something
        	if some condition that holds true most of the time;
	do some other thing;

is already hard to follow, but it is probably excusable in some cases,
because your thought can flow "ah, Ok, these four things are done in
sequence" when you are quickly scanning the code to understand the overall
structure, letting your eyes ignore the "true most of the time" part.

But the following, which is equivalent to what you did, is inexcuable.

	do this;
	do that;
	do something unusual
        	if some condition that rarely holds true;
	do some other thing;

When your eyes and brain are coasting over this segment of code, your
thought process needs to stumble and hiccup at the statment that does
something unusual, and then need to realize that it is qualified with a
statement modifier that says "this is only for rare case".

Written without statement modifier:

	do this;
	do that;
	if (some consition that rarely holds true) {
		do something unusual
        }
	do some other thing;

it is much easier to coast over; you can tell "Ah, after doing this and
that, in the normal case we do some other thing" and do not have to even
look at the details of "something unusual" part.

> -		$smtp->mail( $raw_from ) or die $smtp->message;
> -		$smtp->to( @recipients ) or die $smtp->message;
> -		$smtp->data or die $smtp->message;
> -		$smtp->datasend("$header\n$message") or die $smtp->message;
> -		$smtp->dataend() or die $smtp->message;
> -		$smtp->code =~ /250|200/ or die "Failed to send $subject\n".$smtp->message;
> +		SEND_MAIL:
> +
> +		$smtp->mail($raw_from)               and
> +		$smtp->to(@recipients)               and
> +		$smtp->data                          and
> +		$smtp->datasend("$header\n$message") and
> +		$smtp->dataend                       or
> +
> +		die "Failed to send '$subject': " . $smtp->message . "\n";

These do make things more pleasant to read.