Re: Perl::Critic Tests

[Nick Prater <..hidden..>]

On 2017-04-25 04:12, R. Ransbottom wrote:
> Don't do this, please.
> 
> That is Perl novice thinking which shows a poor understanding of Perl's
> definition--and use of--context.
> 
> The bare return is a basic building block of some standard and
> simpler Perl idioms.
> 
> If we promote bad standards of coding we will discourage the better
> coders from joining in on our projects.

I'm afraid I don't follow. Don't do what? The contributors to this 
thread are not Perl novices. It may not be your intention, but your 
comment reads to me as disparaging and belittling to those that have 
taken part. I don't think that's healthy for our community.

I consider that we should strive for code that is as obvious and 
accessible as possible, even to a novice. That encourages contributions, 
is less prone to errors and more maintainable. The very point of this 
discussion is to promote good standards of coding.

[snip]

> Matt forgot to 'use warnings;'.  Ouch!  Had me going for a minute.
> That example has nothing to do with the bare return statement.

To clarify, Matt didn't forget. He posted a contrived example in an 
online chat, which I then interpreted in my summary. The code you 
commented on is my interpretation of his example. Any deficiencies are 
mine.

The code will still execute even with 'use warnings', so that doesn't 
protect us against such errors.

>> I prefer an explicit "return undef;" as I think it's clearer, more
>> obvious about the intention and less magical. Eric suggests we 
>> problably
>> want to switch from "no return at all" to "return undef".
> 
> The problem with this expression of intent is that it is buried at
> the bottom of a function that someone wants to use not to read.

Is there a better way to express the intent?

> The return statement is designed specifically to be handy when 
> returning
> nothing in list or scalar context.  I suppose the basic example is:
> 
> sub filter_things {
>     if ( $it eq "good" ) {
>         return $it;
>     }
>     return;
> }
> 
> When you use such a function to build up an array the bare return 
> returns
> an empty list.  So you are not building large arrays with useless 
> elements,
> so don't have to skip over them in processing, and so this works:
> 
>     if_or_while ( @array ) { more_to_do(); }
> 
> because you did not return a list with one undefined element but 
> returned
> an empty list.

As a counter-example, what about when building an array of query 
parameters that use undef to represent a NULL value? In these situations 
you do not what those elements to be magically collapsed.

> If you don't use this basic idiom, i.e. the duplex nature of bare 
> return,
> then there is little point to using RequireFinalReturn.  This what a 
> final
> return is meant to do.

RequireFinalReturn makes it obvious what is being returned and ensures 
that you are not inadvertently returning the result of a previous 
expression. An explicit return minimises surprises and makes it easier 
for a human to scan, maintain and debug the code. But that's a different 
policy and a different discussion.

>> I therefore propose that we do not enforce the
>> ProhibitExplicitReturnUndef policy and will add a comment to the tests
> 
> If we are not enforcing the policy we can just drop it from our suite.

The reason for documenting rather than just dropping it is so that 
others, in future, can benefit from our discussion and reasoning, 
without having to start the conversation afresh, duplicate our efforts 
or our mistakes.



Nick


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Ledger-smb-devel mailing list
..hidden..
https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel