Beefy Boxes and Bandwidth Generously Provided by pair Networks DiBona
laziness, impatience, and hubris
 
PerlMonks  

•Re: On goto

by merlyn (Sage)
on Mar 01, 2003 at 01:55 UTC ( #239615=note: print w/ replies, xml ) Need Help??


in reply to On goto

Trivially eliminating the goto, I'd go with:

{ ask('databasetype','Database type?','mysql'); my $found; # aside: I hate " = undef" foreach $cur (@databases) { if (getConf('databasetype') eq $cur) { $found = 1; last; } } unless ($found) { output "Sorry, invalid option\n\n"; redo; } }
Next, I hate the artificial $found variable, so I'd merely move that into a subroutine:
{ ask('databasetype','Database type?','mysql'); unless (is_databasetype_valid()) { output "Sorry, invalid option\n\n"; redo; } } sub is_databasetype_valid { foreach my $cur (@databases) { if (getConf('databasetype') eq $cur) { return 1; } } return 0; }
That'd probably be enough, but that call to getConf seems to be perhaps invariant in that loop, so I'd pull it out to call the subroutine only once during the scan of @databases.

But hopefully, you get the idea now.

-- Randal L. Schwartz, Perl hacker
Be sure to read my standard disclaimer if this is a reply.


Comment on •Re: On goto
Select or Download Code
Re: •Re: On goto
by zachlipton (Beadle) on Mar 01, 2003 at 02:02 UTC
    Wow! Great. After moving the my $found; outside of the block, it works great. Thanks!

    Btw as a side note, why do you hate = undef? Redundant?

      I dislike the "= undef" for two reasons:
      1. It's wrong when applied to initializing empty arrays and hashes, so it doesn't "scale" well.
      2. I presume a knowledge of people reading my code equal to that contained in my Learning Perl book, and in there, it's clear that a new scalar starts with undef

      -- Randal L. Schwartz, Perl hacker
      Be sure to read my standard disclaimer if this is a reply.

        What are your thoughts on using it in constructors of hash or array based objects for purposes of clarity?

        For example:

        sub new { my ($class) = @_; my $this = bless {}, $class; $this->{foo} = undef; $this->{bar} = undef; return $this; }

        Where 'foo' and 'bar' may be initialized during the course of the program, but not at construction. To me, this makes it more clear what an instance of this class consists of. It also can make debugging easier if you are fond of Data::Dumpering as I am.


        "The dead do not recognize context" -- Kai, Lexx
Re^2: On goto
by Aristotle (Chancellor) on Mar 01, 2003 at 13:11 UTC
    { ask('databasetype','Database type?','mysql'); output("Sorry, invalid option\n\n"), redo if not grep getConf('databasetype') eq $_, @databases; }
    Unless @databases is so large that you need the short-circuiting behaviour of the foreach version.

    Makeshifts last the longest.

      That's fine if you're golfing.

      I tend to be a bit more explicit in code that I'm leaving around for someone else to read. That last statement has about six semantic steps that partially nest, partially flow right to left, and partially flow left to right, without the help of denotational punctuation to help me "change gears".

      It's also riddled with booby traps for the maintainer. The parens on output are mandatory, or else the redo would trigger before the function is called. If the expression for the test gets more complicated, or wants to be called only once, there's a bit of gyrating to do.

      I figure anything that takes me more than a dozen seconds to parse needs to be broken up for production code. My personal rule.

      See, if you were just going for compact, I'd slide that even further to:

      output("Sorry, invalid option\n\n") until ask('database','Database type?', 'mysql'), grep getConf('databasetype') eq $_, @databases;
      This gets rid of the naked block entirely.

      But again, I would not use this in production code. This is cute to impress your friends, especially your Lisp-hacking friends. But it really aggravates the maintainer who has to try to grok that at 3am when the production web site crashes.

      -- Randal L. Schwartz, Perl hacker
      Be sure to read my standard disclaimer if this is a reply.

        No, I was not golfing. You're right that it could do with just a hint more punctuation - grep BLOCK would be better than grep EXPR here.

        I disagree that it's hard to understand - only if you've never seen that grep idiom before. Personally I use it all the time, so I don't even have to think about it when I read one anymore.

        I do think it reads naturally:

        Ask   for   a database type   (default is mysql).
        Say "Sorry",   then try again
              if you can't   find   an entry   like   the requested database   in the list of databases.

        I like to choose my code structure so that it maps as closely to a natural language description of what it's trying to do as possible.

        In contrast, I'd never use your no-block example because it violates that principle.

        Makeshifts last the longest.

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others drinking their drinks and smoking their pipes about the Monastery: (10)
As of 2014-04-18 09:26 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    April first is:







    Results (464 votes), past polls