Beefy Boxes and Bandwidth Generously Provided by pair Networks
Welcome to the Monastery
 
PerlMonks  

Re: Lady Aleena's first working module

by Tux (Canon)
on Oct 07, 2009 at 07:39 UTC ( #799670=note: print w/replies, xml ) Need Help??


in reply to Lady Aleena's first working module

You're using one of my pet-peeve style errors:

sub random_alignment { my $type = shift; if ($type eq 'parts') { return $parts[rand @parts]; } elsif ($type eq 'good_vs_evil') { return $good_vs_evil[rand @good_vs_evil]; } elsif ($type eq 'lawful_vs_chaotic') { return $lawful_vs_chaotic[rand @lawful_vs_chaotic]; }

After a return there can never be an else. the sub returns, so this much better reads like this:

sub random_alignment { my $type = shift; if ($type eq 'parts') { return $parts[rand @parts]; } if ($type eq 'good_vs_evil') { return $good_vs_evil[rand @good_vs_evil]; } if ($type eq 'lawful_vs_chaotic') { return $lawful_vs_chaotic[rand @lawful_vs_chaotic]; }

And personally, I'd go even further:

sub random_alignment { my $type = shift; $type eq "parts" and return $parts[rand @parts]; $type eq "good_vs_evil" and return $good_vs_evil[rand @good_vs_ +evil]; $type eq "lawful_vs_chaotic" and return $lawful_vs_chaotic[rand @law +ful_vs_chaotic];

Clean, short code. Beautiful, right?


Enjoy, Have FUN! H.Merijn

Replies are listed 'Best First'.
Re^2: Lady Aleena's first working module
by Herkum (Parson) on Oct 07, 2009 at 14:34 UTC

    Now you are doing one of my pet peeves, using to much syntax which is not clarifying the code at all. This is better,

    my $type = shift; return $type eq 'parts' ? $parts[ rand @parts] : $type eq 'good_vs_evil' ? $good_vs_evil[ rand @good_v +s_eval] : $type eq 'lawful_vs_chaotic' ? $lawful_vs_chaotic[ rand @lawful +_vs_chaotic] ? q{}

      That kind of code will only work of there is just one return, and "" (or the hideous q{} from PBP) is the only possible alternative.
      My example just simplifies the pre-emptive returns, and lives in the assumption that the default (after all the returns were done) still needs some real coding.

      Personally, I like brevity, and both my code and yours are terse. I do not however think that your code is any clearer than mine. Both are clearer than the code of the OP though.


      Enjoy, Have FUN! H.Merijn
Re^2: Lady Aleena's first working module
by Lady_Aleena (Curate) on Oct 14, 2009 at 21:55 UTC
    Tux,

    With what Herkum suggested in Re: Lady Aleena's first working module, using a hash instead of a lot of arrays, I was able to cut down on the redundant code a bit. You can see my changes in Re^2: Lady Aleena's first working module. I am a bit confused as to why you would put the closing bracket of the if on the next level down from it. I am of the mind that the closing bracket should be on the same level as where it was opened. I am also not a big fan of condensing the if, but that is because I don't want it to get lost. That is the same reason I concatenate over interpolate.

    Have a nice day!
    Lady Aleena

      In my humble but very honest opinion, indenting the closing brace to that level is the only logical thing to do. I have tried to explain that (and some other style issues) in here. I know many won't agree, but you have to admit I at least gave it a lot of thought and I'm not one of those people that just blindly follow what their favourite editor thinks to be the best style today.


      Enjoy, Have FUN! H.Merijn

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others pondering the Monastery: (8)
As of 2021-04-19 07:28 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found

    Notices?