Beefy Boxes and Bandwidth Generously Provided by pair Networks
No such thing as a small change
 
PerlMonks  

Re^2: Module Announcement: Perl-Critic-1.01 (just a scalar)

by tye (Cardinal)
on Jan 26, 2007 at 20:55 UTC ( #596790=note: print w/ replies, xml ) Need Help??


in reply to Re: Module Announcement: Perl-Critic-1.01
in thread Module Announcement: Perl-Critic-1.01

If your function normally returns scalars, then doing return; is likely the wrong thing to do. If your function at least sometimes returns lists, then return undef; is likely the wrong thing to do. The latter is covered by other replies.

The reason that the former is wrong is because it can break code like:

my %hash= ( key1 => genValue1(), key2 => genValue2() }; # %hash= ( key1 => 'key2' ); is never what you want to get

And the problems with return undef; don't really apply to functions that only ever return a scalar because you don't write code like:

if( @array= getScalar() ) { ... } else { die "Couldn't get scalar"; }

So, unless Perl::Critic checks whether your function ever returns other than 1 scalar elsewhere, I consider this warning to be encouraging a bad practice (rather than just pushing a reasonable practice too hard as many people will find many of the possible warnings, surely).

- tye        


Comment on Re^2: Module Announcement: Perl-Critic-1.01 (just a scalar)
Select or Download Code
Re^3: Module Announcement: Perl-Critic-1.01 (just a scalar)
by chromatic (Archbishop) on Jan 26, 2007 at 22:07 UTC
    The reason that the former is wrong is because it can break code like:

    Why do you not consider the hash assignment code broken? If you need to call a function in scalar context, make it explicit.

      Why make (key => scalar &functionScalar) explicit, but not return undef;? That sounds like a matter of personal preference to me.

      -Paul

        Because it's the calling context that matters!

        Bare return; does the right thing in both scalar and list contexts. In my mind, that's a good thing.

        If you find yourself in an ambiguous situation, then you disambigate by being explicit. It would be nice if there were never ambiguous code, but that's not realistic.

      Do you write the following?

      my %hash= ( key1 => scalar lc $value );

      If not, then it is probably because you know that 'lc' just returns a scalar. If you do, then there are probably a lot of people who find you silly. 'lc' isn't the only function I use that just returns scalars. Sprinkling 'scalar' all over hellenbach doesn't improve code much in my experience.

      I actually consider the => to be broken for not enforcing scalar context on both sides. But that still just shifts the problem to things like function arguments.

      Now, there are places where using scalar() is more appropriate. But, no, I don't consider using it on a function that just returns a scalar to be such a place. And I don't think transforming every function that might want to return an undefined scalar into "a function that returns a scalar except when it wants to return a undefined scalar in which case it returns an empty list instead and if you really wanted the undefined scalar then you should wrap the call in scalar() for that purpose" to be even close to an improvement strategy.

      - tye        

        Your position is problematic because your model ignores a fundamental, from perlsub: The Perl model for function ... return values is simple: ... all functions ... return to their caller one single flat list of scalars.

        lc doesn't return a scalar it returns a one element list.

        Be well,
        rir

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://596790]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others about the Monastery: (13)
As of 2014-10-21 18:47 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    For retirement, I am banking on:










    Results (106 votes), past polls