Re: Perl::Critic Tests

["R. Ransbottom" <..hidden..>]

On Tue, Apr 25, 2017 at 05:47:42PM +0100, Nick Prater wrote:
> 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

> 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 
> ...

My apologies for brutishness and obscureness.  I appreciate the kindness 
of your rebuke.  I was disparaging the thinking; if I thought poorly 
of skills of the people involved I would not be advocating for standard 
practices that embrace more nuanced code rather I would push for practices 
that disallow them for safety's sake.

Please don't do this:  not "enforce the ProhibitExplicitReturnUndef policy
and add a comment to the tests to explain why."

> 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.

I don't agree entirely.  I  don't think a complex sql statement should 
be simplified for me as a beginner.  I would prefer that good journeyman
code be written--that good code is modeled for me if I care to learn.
To a beginner, the obviously complex code is less dangerous than the code
that seems straightforward but is not.

I think the simple "return undef" models a bad practice.  It is certainly
reasonable to write that in the  situation when undefs have to
be treated as data values headed to or from SQL.

The choices seem to be:

    return undef;   # no change; treat as acceptable 
    undef;  ## no critic
    return undef;  ## no critic
    return scalar undef;   
    die "bad context blah blah" if wantarray; return;
    caller_deals_with_normal_return_magic( returner());


If making a change, I would suggest the 'return scalar undef' which puts the
solution in code.  I expresses the intent.  I don't think any of the first
four choices expresses the intent clearly for novices.

The last two are logic changes so I rule them out for refactoring 
working code.

> > 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 

That was supposed to be a device, but being too-literal myself I should 
have crafted it better.  I did not think Matt forgot; but for me warnings
have trivialized the problem to extent that I haven't thought about it.

>  ...

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

I find 'use warnings' helpful.  I find a more draconian or nagging
mechanism helpful too.

> >> 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?

    caller_deals_with_normal_return_magic( returner());

would be the best.

That all such callee's be private subs would be an amelioration.

> 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.

I looked at about 15 return undefs in the code but didn't find code
that looked like it was used toward this purpose.  

> The reason for documenting rather than just dropping it is so that 
> others, in future, can benefit from our discussion and reasoning, 

And hopefully our subsequent actions.

Thanks,
Rob

------------------------------------------------------------------------------
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