On Tue, Apr 25, 2017 at 5:12 AM, R. Ransbottom <rir...@comcast.net> 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.
>
My interpretation of it all is that Matt claims (and I fully understand his
position) that these simpler idioms are a source for bugs because the
resulting program may not do what the unsuspecting programmer thinks it
does, especially in the context of returning empty lists.
> If we promote bad standards of coding we will discourage the better
> coders from joining in on our projects.
>
True, but elegant code which has a high(er) risk of being insecure can't be
construed as "better code", can it? The programmers we'd like to attract on
the project would be those who put secure coding over everything else. In
my book that means that they write clear and concise code.
The second characteristic we'd look for in people who want to join is that
they (want to learn to) love to write tests for the code they deliver. (And
so on, and so on...)
> On Sat, Apr 22, 2017 at 07:37:25AM +0100, Nick Prater wrote:
>
> > Matt Trout raised an issue that this policy leads to surprising
>
> > sub get_value {
> > return;
> > }
> > my %hash = (
> > row => get_value(),
> > id => 1
> > );
> > print Dumper \%hash;
> > ----
> >
> > which returns:
> > $VAR1 = {
> > '1' => undef,
> > 'row' => 'id'
> > };
>
> Matt forgot to 'use warnings;'. Ouch! Had me going for a minute.
> That example has nothing to do with the bare return statement.
>
actually, it does have to do with a bare return statement, because Perl
decides itself to make the return value `()`. I'm with Nick here: the code
did run to get the warning, meaning that the damage has already been done.
Warnings don't prevent code from being unsecure. They might help to detect
cases after the fact thought.
> Witness:
>
> %h = ( "george" );
>
> gives "george" => undef and warns
>
> > 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.
>
The point here was that for clarity of code, it's best to have a function
either return an array/list or a scalar. While returning "one or the other
based on context" results in some elegant code, I'm with Matt that it does
not solve the problem of casting values from scalars to arrays and back in
a program of moderate size. But if it doesn't, then maybe it's better to
have clear intentions all the way.
As for "burried at the bottom", I'm thinking that it should be clearly
stated in the API documentation: the place to look for any user of an API
function. So, I hope that given enough time there won't be a need to read
all of our code in the code base in order to be able to extend LedgerSMB.
Regards,
Erik.
> 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.
>
> 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.
>
> > 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.
>
> > Thanks to k-man and dcg_mx for initiating and sharing the discussion
> > with Matt Trout.
>
> 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
> Ledger-smb-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel
>
--
Bye,
Erik.
http://efficito.com -- Hosted accounting and ERP.
Robust and Flexible. No vendor lock-in.
------------------------------------------------------------------------------
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
Ledger-smb-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ledger-smb-devel
