Beefy Boxes and Bandwidth Generously Provided by pair Networks
go ahead... be a heretic
 
PerlMonks  

Wrong idioms

by McA (Curate)
on Mar 29, 2013 at 19:56 UTC ( #1026189=perlquestion: print w/ replies, xml ) Need Help??
McA has asked for the wisdom of the Perl Monks concerning the following question:

Hi all,

digging in the code of a popular web framework (yes, yes, learning by reading) I found an equivalent to the following snippet:

sub readfile { my $filename = shift or return; ... }

When you look at CPAN code you see this kind of idiom often. But with this idiom you declare a string of "0" as bad input. In this case: Why shouldn't I have a file named "0"? Sometimes I get the feeling that simply the look and feel of the code is the reason to do so. Ignoring the //-operator (starting with 5.10) you would have to write the following to allow a string "0":

my $filename = shift; return unless(defined $filename && $filename ne '');
This is really not so fancy like the above
my $filename = shift or return;

So, my question: Do you think the programmers are not aware of the subtle difference? Do you think they know it but they like the fancy code more accepting to declare a valid input as bad? I have a bad feeling seeing problematic idioms cemented by popular code. What do you think?

Sidenote: The //-operator was a step in the right direction, but IMHO an operator like in bash (-n $filename) is missing.

McA

Comment on Wrong idioms
Select or Download Code
Re: Wrong idioms
by moritz (Cardinal) on Mar 29, 2013 at 21:00 UTC
    Sometimes I get the feeling that simply the look and feel of the code is the reason to do so.

    Or maybe just the effort to write the code in the first place. Or missing awareness.

    There are many more similar but less obvious cases. For example code that interpolates an array into a double-quoted string depends on the value of $". Code that uses print implicitly uses $\ and $,, and if you use readline (or <$fh>) or chomp you'd better hope nobody messed up your $.

    And good thing that $[ is deprecated.

    And your seemingly harmless loop while (<>) { } gets an entirely new meaning if $^I is set.

    Speaking of <>, did you know that it executes code for you?

    $ perl -we 'while (<>) { print }' 'fortune |' To downgrade the human mind is bad theology. -- C. K. Chesterton

    So you see there is a whole scale of things you should consider when you want to write truely robust and correct code. But once you start to write your code as

    sub do_something { my ($filename, @rest) = @_; return unless defined $filename; # ensure basic sanity: local $/ = "\n"; local $, = ''; local $\ = ''; local $" = ' '; local $@; # we might want to use eval { } }

    you get a lot of boilerplate code, and writing Perl stops to be fun.

    So you can either leave Perl 5 for good, or decide where on your scale between fun and correctness your code should be. Don't condemn anybody for using a simple truth check instead of defined, but not for failing to reset all used global variables in each routine that could be called from the outside.

    Note that Perl 6 drastically reduces the number of global and special variables, and most built-in functions don't depend on them to the same degree as in Perl 5.

      Thank your for your comment and insights. Yes, my example is only the tip of the iceberg as you showed very clearly concerning robustness. But I think there is a little difference between allowing a filename of "0" and messing up a default behaviour of perl by changing special variables.

      But anyways: Be sure I don't condemn anybody for how he or she is coding. The only person I do is myself.

      And you have to admit that I haven't used the term error anywhere because it's the right of the programmer to write a function where "0" is not a valid filename in his sence.

      I initiated this thread because on the one handside newcomers are said to study at least top rated CPAN code to learn from it and on the other handside is a framework like a fundament which shouldn't have litte fissures.

      McA

      P.S.: A ++ for showing all the implications while coding in perl5.

      I think if you try to submit a bug report for a module, saying that it works wrong when you redefine several perl special variables, nobody will agree to fix that. At maximum they will add a note to the specification, that you should not redefine variables.

      But bug with filename '0' is different case. It's a bug, absolutely.

        I think if you try to submit a bug report for a module, saying that it works wrong when you redefine several perl special variables, nobody will agree to fix that.

        But why? It's not wrong to assign a value to those special variables. That's what they are for. If you don't ever assign values to them, there's no reason they'd need to exist at all.

        So, why would somebody reject such a bug report? Maybe because it's unsual that it's done in code that isn't a oneliner.

        But bug with filename '0' is different case. It's a bug, absolutely.

        But one can argue that using 0 as a file name is also unusual.

        What do you think about modules that can't handle file names that contain quotes? Or a newline? Is that also a bug, absolutely? What about not handling non-ASCII file names (for which there seems to be no cross-platform way of handling them corectly)?

        You draw a line and say "not handling non-default $, is not a bug", but you say "not handling 0 filenames is a bug". Based on what criteria do you draw that line? And where do you draw it?

        For the record, I too would consider not handling a filename of 0 a bug, but I lack your perspective on which corner cases need to be handled, and which are OK to ignore.

      If you mess with these variables, it's YOUR duty to put them back in order! The code that changes them and leaves them changed is incorrect, not the code that fails to check and/or reset them.

      Jenda
      Enoch was right!
      Enjoy the last years of Rome.

        Well, if you mess with your caller's variables, I agree. But if you want to use those variables for your own purposes, you can't help but let the callee also see the changed variables (because you can only localize the, not modify them in a lexical scope).

Re: Wrong idioms (lazyness)
by Anonymous Monk on Mar 29, 2013 at 21:54 UTC

    Look at the amount of code the author writes, I don't think its a problem of awareness

    Either way, file a bug report :)

      It's a bug when code doesn't do what specifications says. That means - missing any specification - in this case I can at most file an enhancement proposal... :-)

      I really respect what the people are doing.

        That means - missing any specification - in this case I can at most file an enhancement proposal... :-)

        Um, no, obvious function is obvious, obvious bug is obvious

        read_file_content() doesn't need to specify limits on filename, there should be zero

Re: Wrong idioms
by Lotus1 (Chaplain) on Mar 29, 2013 at 23:16 UTC

    ++ for mentioning the -n Bash operator, I wasn't aware of the string testing operators in Bash. I was shocked to find out that Bash uses == for strings and -eq for integers.

    I found a stackoverflow node that deals with the empty string issue and the best answer for Perl seemed to be to use the length function.

    return unless length $str;

      Lotus1 wrote

      I found a stackoverflow node that deals with the empty string issue and the best answer for Perl seemed to be to use the length function.

      IMHO, probably so! And FWIW, Perl is not the only programming language in which some obvious problems pertaining to "truth" might need to be thought about carefully. We've discussed Bash, and here's another one: VimL or Vim script (AKA “ex”).

      In VimL it is complicated where Bash makes it easy. To test for both defined-ness and non-null-ness / non-zero-ness, there are some hoops to jump through.

      Vim will complain if we try to do anything with a variable identifier that has not been defined yet. So I found myself having to write a function that does this, since I was rewriting the same code so often in my .vimrc:

      # Vim script language function! EnNZ(kipper) if !exists(a:kipper) | return 0 | endif exec 'let arg_ref = '.a:kipper if ( strlen(arg_ref) || arg_ref != 0 ) return 1 endi return 0 endfunction

      First a test must be run to determine if the referenced variable even exists. Then if it does we need to "extract" the reference (ok, "dereference" if you like). Then we can finally test the data actually in the variable, using "length" (but in VimL we spell it strlen() as if we were C programmers), and also accounting for a numeric value by following that test with a numerical comparison. The end result is that we've worked out whether this variable is approximately "true" or "false" in a Perlish sense.

      This is not a Perl topic ... but improvements, critiques welcomed (I know there are Vimmers hiding in plain sight around here).

      Oh, and BTW: anyone who went Ewwww when they perceived the design principles of VimL revealed in that code: you wouldn't be the first ;-)

      Intrepid Mar 31, 2013 at 18:26 UTC
      Examine what is said, not who speaks.
      Love the truth but pardon error.
      Silence betokens consent.
      In the absence of evidence, opinion is indistinguishable from prejudice.
Re: Wrong idioms
by BrowserUk (Pope) on Mar 29, 2013 at 23:39 UTC

    You've noticed a problem, but are proposing the wrong solution.

    The author wasn't being lazy, but overzealous.

    The better solution is to simply skip the check of the filename completely.

    Pass the filename supplied to the open function and let it detect and report the error.

    This approach is portable because it lets the OS decide whether the string passed is valid.

    The alternative is for every file handling routine to detect the current OS and then replicate all of the complex rules for filenames and paths for all systems; and that is a nonsense.


    With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    "Science is about questioning the status quo. Questioning authority".
    In the absence of evidence, opinion is indistinguishable from prejudice.

      the problem with that is undef opens a temporary file

      :) its http://search.cpan.org/perldoc/Dancer::FileUtils#read_file_content

      sub open_file { my ( $mode, $filename ) = @_; open my $fh, $mode, $filename or raise core_fileutils => "$! while opening '$filename' using m +ode '$mode'"; return set_file_mode($fh); } sub read_file_content { my $file = shift or return; my $fh = open_file( '<', $file ); return wantarray ? read_glob_content($fh) : scalar read_glob_content($fh); }

        This will also "open" file without error, if it's not a file, but a directory. Also, does this have any sense?
        return wantarray ? read_glob_content($fh) : scalar read_glob_content($fh);
        In case we don't want an array, it will be converted to 'scalar' anyway.
        the problem with that is undef opens a temporary file

        And what is to say that is not both a useful feature and exactly what the user intends?


        With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.
      Maybe in TS example, readfile should return undef/empty list in case called without arguments ?
Re: Wrong idioms
by vsespb (Hermit) on Mar 30, 2013 at 00:11 UTC

    Personally I see a correlation between bugs and module popularity.

    More popularity = More new features implemented fast = no time spent in refactoring/bugfixing = lots of technical debt (http://en.wikipedia.org/wiki/Technical_debt) = more bugs

    Or, for example, on GitHub, each project Fork is as good for search ranking as maybe 10 Stars. So more stupid annoying long standing bugs ( which can be fixed in one line) + less unit tests = more Forks = more popularity

    Also, if you ever finish your module with all needed functionality, without bugs and without making it bloatware. You won't have any more work to do, and people will think the module is "not supported any more", because "it's not updated several months". = no popularity

Re: Wrong idioms
by perl-diddler (Hermit) on Mar 30, 2013 at 00:46 UTC
    The above is, perhaps, an unlikely situation... but the bash equiv of -n would be length($filename)....

    Does that do what you want?

Re: Wrong idioms
by educated_foo (Vicar) on Mar 30, 2013 at 02:40 UTC
    Why shouldn't I have a file named "0"?
    Just as a side note... Regardless of correctness, "don't name your files with obnoxious names" is good advice if you want to avoid problems with others' software. "0" is kind of a corner case, but if you stick to alphanumerics, '-', and '_', you're less likely to have to fix other people's stuff. On the one hand, you're "technically correct" using all sorts of filenames. On the other hand, you're just asking for trouble.
      Hm, I think '-' can cause problems with getopts sometimes. Also - what other languages treat '0' as false?

        "Also - what other languages treat '0' as false?"

        It would be horrible if "0" were treated as true, given that the integer 0 is false and Perl silently converts between strings and numbers.

        What other languages? PHP does.

        package Cow { use Moo; has name => (is => 'lazy', default => sub { 'Mooington' }) } say Cow->new->name
        Update: Post recomposed for better readability

        > Also - what other languages treat "0" as false?

      • JavaScript

        Num yes Str no

        >>> !! 0 false >>> !! "0" true

      • Python

        Num yes Str no

        >>> not not 0 False >>> not not "0" True

      • Ruby

        Num no Str no

        irb(main):002:0> !! 0 => true irb(main):003:0> !! "0" (irb):3: warning: string literal in condition => true

      • Perl

        Num yes Str yes

        DB<102> !! 0 => "" DB<103> !! "0" => ""

        Cheers Rolf

        ( addicted to the Perl Programming Language)

        Hm, I think '-' can cause problems with getopts sometimes.
        Oops! Starting filenames with '-' is just asking for trouble. Your life will be least painful if you start names with [a-z_], use only one '.', don't make them too long, and don't rely on case differences alone. If you find yourself wanting arbitrary filenames, either never pass them to other software, or consider using a database.
Re: Wrong idioms
by vsespb (Hermit) on Mar 30, 2013 at 09:43 UTC
    sub readfile { my $filename = shift or return; ... }
    If this code should only check that filename is defined (i.e. allow empty string), you can write (even for perl <5.10) it like this:
    defined(my $filename = shift) or return;
Re: Wrong idioms
by Jenda (Abbot) on Apr 01, 2013 at 10:48 UTC

    Now does or does the framework not require that the extension of the filename is something specific? "0.tmpl" doesn't evaluate to false.

    Besides why should you have a file named "0"?

    Jenda
    Enoch was right!
    Enjoy the last years of Rome.

      Besides why should you have a file named "0"?

      Because that's what got sent. Or because it is one of a series.

      Or more simply, why not?


      With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
      Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
      "Science is about questioning the status quo. Questioning authority".
      In the absence of evidence, opinion is indistinguishable from prejudice.

        Does the idiom only apply to filenames? For a directory name, "0" would be not too esoteric.

      Besides why should you have a file named "0"?

      Why should Jenda be Jenda?

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://1026189]
Approved by marto
Front-paged by Old_Gray_Bear
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others studying the Monastery: (14)
As of 2014-08-28 13:10 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (260 votes), past polls