Anonymous Monk has asked for the wisdom of the Perl Monks concerning the following question:

I am calling a Function on conditions. Everything works fine but I like your Perl Monks site knowledge to find if there is a better way of doing this and making it look better and be more efficient? Please advise.
if(($ma == 1) && ($sy == 1)) { if($al == 1) { print "\nshow some text.\n"; notify(); } elsif($tr == 1) { print "\nnext data info\n"; notify(); } elsif($ge == 1) { print "\nnew info here\n"; notify(); } }

Replies are listed 'Best First'.
Re: Conditions and Function call
by dragonchild (Archbishop) on Feb 23, 2004 at 18:19 UTC
    Yes, there are many other ways of doing this. Now, better is up to you (and the people who will be maintaining your code a year from now).

    A few comments (in no particular order)

    • Your variable names are horrible. You're not paying per keystroke, nor are you charged per byte in your source file. The compiler doesn't care, but your maintainer will, so spend some time coming up with more descriptive variable names.
    • You seem to be calling the same function without passing any parameters in, so it's not clear exactly what kind of decisions you're making. Also, it's not clear what the context is around this snippet.

    Before I give alternatives, I'm going to make a few assumptions:

    1. You are running this snippet in some loop. The various parameters can change, depending on conditions that the loop iteration will discover
    2. notify() (or whatever function(s) you might need to call) do not accept parameters.
    3. All these parameters are boolean (true or false).

    Some options:

    • while (1) { my %options = discover_my_options(); next unless $options{ma} && $options{sy}; if ($options{a1}) { do_a1(); next; } if ($options{tr}) { do_tr(); next; } if ($options{ge}) { do_ge(); next; } }
    • my %dispatch_table = ( a1 => \&do_a1, tr => \&do_ge, ge => \&do_tr, ); while (1) { my ($skip_iteration, $value) = discover_my_options(); next if $skip_iteration; my $function = $dispatch_table{$value} || next; $function->(); }

    The first is a cleanup of your existing the code. The second is a way of parameterizing function calls. This isn't something only Perl can do. MUD code has been doing this since the late 80's, and that was (usually) written in C, using funcp's.

    ------
    We are the carpenters and bricklayers of the Information Age.

    Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.

Re: Conditions and Function call
by waswas-fng (Curate) on Feb 23, 2004 at 18:21 UTC
Re: Conditions and Function call
by KPeter0314 (Deacon) on Feb 23, 2004 at 18:10 UTC
    Not really shorter, but possibly cleaner depending on what the logic you were intending:
    if(($ma == 1) && ($sy == 1)) { SWITCH: { $al == 1 && do { print "\nshow some text.\n"; notify(); last SWITCH; }; $tr == 1 && do { print "\nnext data info\n"; notify(); last SWITCH; }; $ge == 1 && do { print "\nnew info here\n"; notify(); last SWITCH; }; }; };
    I will say that until you get more comfortable with Perl or are really trying for Perl golf, then I would suggest readablity over short.

    -Kurt

Re: Conditions and Function call
by Anonymous Monk on Feb 25, 2004 at 01:36 UTC
    Let me take a moment to strongly back up the statement of the preceeding poster regarding how crap those variable names are, and then further indicate that the actual code structure suggests a finite-state *nightmare* in your code.

    You can tell when you have one of those, when your elseif clause operates solely on different variables. Hell I'd probably even go so far as to say most situations in which you use any other variable in the elseif is a sign of bad structure.

    I suggest you get the whole of your code out, or at least an easily-readable section of it, and post up here for commentary, because even using the best of the solutions posted here, your structure is still going to cause you serious problems.