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

Flag variables

by oakbox (Chaplain)
on Mar 03, 2003 at 10:47 UTC ( [id://239992]=perlquestion: print w/replies, xml ) Need Help??

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

A few weeks ago I was reading a list of 'don'ts' in perl. One of the things to avoid is the following:

my $flag = 0; foreach my $var (@somearray){ if($var eq "foo"){ $flag = 1; last;} } if($flag eq "1"){ &someaction;}

I do this kind of thing All The Time and I don't know a) Why is it wrong? and b) How else would I go about it?

Checking if $var is equal to "foo" might be some other, more complex, calculation or maybe a database call of some kind.

oakbox

Replies are listed 'Best First'.
Re: Flag variables
by zby (Vicar) on Mar 03, 2003 at 10:53 UTC
    How about:
    foreach my $var (@somearray){ if($var eq "foo"){ &someaction; last;} }
Re: Flag variables
by robartes (Priest) on Mar 03, 2003 at 10:57 UTC
    I would say it detracts from code clarity and locality (sp?): your someaction depends on some other piece of code (the conditional setting of $flag) which could be several lines away. If all you want to do is someaction, the $flag is totally superfluous - just do the action directly:
    foreach my $var (@somearray){ if($var eq "foo"){ someaction(); last;} }
    More learned minds than mine will undoubtedly come up with other reasons why flag variables are often "bad".

    CU
    Robartes-

Re: Flag variables
by Aristotle (Chancellor) on Mar 03, 2003 at 13:26 UTC

    You might be interested in Mark-Jason Dominus' excellent Program Repair Shop and Red Flags article series on Perl.com.

    In a pinch, you should avoid flag variables wherever possible - they do nothing to clarify the algorithm and can often obscure it.

    Makeshifts last the longest.

Re: Flag variables
by adrianh (Chancellor) on Mar 03, 2003 at 13:49 UTC

    The "problem" is the gap between where the flag is set and where it is used. This action at a distance can make code far harder to understand and maintain. Code rewrites can move the action a long way from the condition that sets the flag making things even worse. You can very quickly end up with a mess of flags and conditions sprinkled throughout your code.

    (as a separate style point I would not use "eq" to test for a boolean flag, just do someaction() if $flag;. Also use of & to call subroutines tends to be frowned upon because of the implicit argument passing.)

    If the condition was cheap, I'd just use grep as blakem did.

    If the condition is expensive I'd use first from List::Util, which allows you to say:

    someaction() if first { $_ eq "foo } @somearray

    This keeps the trigger condition and action close together, and doesn't do unnecessary checks once a match is found.

Re: Flag variables
by gmax (Abbot) on Mar 03, 2003 at 12:20 UTC

    As a significant example of why flags can be messy, check this particularly thorny source code.

    I needed to use that module and I found out that it was faulty. I tried to fix it, but its logic eluded me. I contacted the author, who didn't do anything, and therefore I wrote my own parser using a rather different approach.

    This module is supposed to parse chess games. The problem in this code is that it skips the last game in each file it parses. I was able to spot where the problem is. As for fixing it using the same labyrinth of flags system, I simply couldn't.

    ## ## WARNING! broken code ahead! ## my $FLAG=1; my $GAME=0; ## ## ... snip ## sub ReadGame { my $self=shift; my $continue=1; $self->_init(); do { if ( $continue == 2 ) { $continue = 0 } if ( $LINE =~/\[(.*)\]/ ) { my $line=$1; my ($key,$val,$junk)=split(/"/,$line); $key=~s/ $//g, $self->{$key}=$val; $GAME=0; } elsif ( $LINE !~/^$/) { $self->{Game}.=$LINE; $GAME=1; } elsif ( $GAME == 1 ) { $FLAG=0; } $LINE=<FICPGN>; if ( eof(FICPGN) && $continue == 1 ) { $continue = 2 } $LINE=~s/\r//; } while ( $FLAG==1 ); return ( $continue ) ; }
    This code uses not one but THREE flags, transforming a would-be OOP module into a peculiar example of spaghetti code.
    _ _ _ _ (_|| | |(_|>< _|
Re: Flag variables
by Anonymous Monk on Mar 03, 2003 at 12:45 UTC
    Problems. Poor locality of reference. Possibility that the value you set and check are unrelated. Meaningless variable name making it hard to know what the flag means - people often write code thinking the flag means X when it is true, and then slip up and treat it as NOT X instead.

    Not all flags are bad. But make the flag's name a yes/no question which its value is the answer to (avoiding reversing its meaning), and don't use a flag where an alternative suggests itself.

    Additional misfeature in your code. The &foo notation passes implicit arguments which is often not desired. As perlsub says, This is an efficiency mechanism that new users may wish to avoid. *ahem* Experienced users as well. :-)

Re: Flag variables
by blakem (Monsignor) on Mar 03, 2003 at 10:58 UTC
    How about:
    if (grep {$_ eq "foo"} @somearray) { someaction(); }
    Or even:
    someaction() if grep {$_ eq "foo"} @somearray;

    -Blake

      Though it is always good to use built-ins like "grep" whenever possible, in this case there is a difference between this code and the original: In the original, once the test succeeds, it is no longer performed (i.e. last is called), whereas here the test will be performed on every item in the list. If this is a simple equality test, that is not so bad, but if it is something like a database lookup, and the list is long, that is a lot of extra overhead.

      Now if grep were smart enough to shortcut when called in "boolean" context, then there would be no reason not to use it. Of course, "boolean" context per se won't exist until perl 6...

      --JAS
        Now if grep were smart enough to shortcut when called in "boolean" context, then there would be no reason not to use it. Of course, "boolean" context per se won't exist until perl 6...

        Until then, using first from List::Util is one alternative.

Re: Flag variables
by Bilbo (Pilgrim) on Mar 03, 2003 at 10:54 UTC

    I don't know why this would be wrong, but I can see an alternative way of doing this:

    foreach my $var (@somearray){ if($var eq "foo"){ &someaction; last; } }

    Update: sorry for the duplicate reply - zby beat me to it

Re: Flag variables
by arturo (Vicar) on Mar 03, 2003 at 14:04 UTC

    In addition to the locality issue (where is $flag set vs. where it's eventually tested), it's also not idiomatic Perl. Assuming you *were* going to use a flag variable, it's nice to use Perl's notion of truth:

    my $foundFoo =0; foreach my $var( @somearray) { if ($var eq 'foo') { $foundFoo =1; last; } } if ( $foundFoo ) { someaction(); }

    And let me also repeat the warning that &action(); does not mean the same as action();; prepending the ampersand, among other things, passes the current value of the @_ array as arguments to action. See perlsub for more information about stuff like that. update a comment by PodMaster alerted me to something here that may be misleading: &foo; is not the same as &foo();, in that the second will not pass @_ to foo, but the two forms still have different semantics; &foo() will call a user-defined sub over a builtin one (with an empty list of arguments), whereas foo() will not.

    HTH

    If not P, what? Q maybe?
    "Sidney Morgenbesser"

Re: Flag variables (to represent state)
by grinder (Bishop) on Mar 03, 2003 at 16:13 UTC

    Interesting question, I've never given it much consideration and I'm guilty of using somthing like it. Consider a snippet I wrote recently: Ordering hash replacements to avoid clobbering things (update chaining). At one point I check whether two arrays are equal.

    In other words, @one = qw/a b c/; @two = qw/a b c/; die if @one == @two; (where == in this instance is a deep equality: each element is compared in turn (and thus will die with these inputs)). As soon as two elements differ, I know the arrays can't be the same, so I can bail out early.

    The code looks like:

    my $is_loop = 1; for( my $n = 0; $n < scalar @loop_keys; ++$n ) { if( $loop_keys[$n] ne $loop_vals[$n] ) { $is_loop = 0; last; } } die if $is_loop;

    I suppose there's a more compact way if saying that, but I pondered the question for about 1.38 seconds before banging out the above code. I assume perl 6's hyper operator is going to be of great help to me in this respect.

    Note that I'm actually doing the opposite: seeing whether the flag still has the same value, not whether it has changed.

    I often resort to this type of algorithm. It's like setting up a hypothesis at the beginning of the loop and seeing whether it can be disproved. The flag variables always seem to be named $found or $is_foo. In this particular case, it's easy to see whether the two are different. As soon as one pair fails to line up, you know they can't be equal, hence you can leave early (in the particular domain this code comes from it's usually the first iteration).

    update: a few wordos and typos fixed

    Another update: As IlyaM points out, there are better ways of doing the above example. That's the problem with contrived examples. But the idea still stands that you start in a certain condition, do a whole lot of stuff and then see if you remained in the initial condition.

    You might move out of the condition quickly, or not at all. It's more the idea of short-circuiting a possibly lengthy examination... in the worst case you will have to examine every single element anyway, but if you can bail out early, so much the better.


    print@_{sort keys %_},$/if%_=split//,'= & *a?b:e\f/h^h!j+n,o@o;r$s-t%t#u'
      Why not just?
      for( my $n = 0; $n < scalar @loop_keys; ++$n ) { if( $loop_keys[$n] ne $loop_vals[$n] ) { die; } }
      or more Perl-ish
      for my $n ( 0 .. @loop_keys - 1 ) { if( $loop_keys[$n] ne $loop_vals[$n] ) { die; } }

      --
      Ilya Martynov, ilya@iponweb.net
      CTO IPonWEB (UK) Ltd
      Quality Perl Programming and Unix Support UK managed @ offshore prices - http://www.iponweb.net
      Personal website - http://martynov.org

Re: Flag variables
by KPeter0314 (Deacon) on Mar 03, 2003 at 15:35 UTC
    Not to be a troublesome monk, but I have a twist on the Wisdom that oakbox seeks. I too being just a learning monk with nobody local to learn from.

    Given that the code in the original post is not what we should aspire to, what about looking for multiple matching items in the array? Say for instance that the array is not unique and you want to look for an instance of "foo" and also "bar". Either of these may show up once, many times, or even never. In any case we only wan to run the resulting action once

    Would the code from blakem be the best? I hadn't used the grep fuction in this way and am glad to be illumined to it. It would seem to be expensive though if @somearray was rather large to iterate through it multiple times.

    fooaction() if grep { $_ eq "foo" } @somearray; baraction() if grep { $_ eq "bar" } @somearray;
    Or, would there be a better way than expanding the code to this if the @somearray were large:
    my $fooflag = 0; my $barflag = 0; foreach my $var (@somearray){ if($var eq "foo"){ $fooflag = 1;} if($var eq "bar"){ $barflag = 1;} } if($fooflag eq "1"){ &fooaction;} if($barflag eq "1"){ &baraction;}
    Note: you wouldn't want to jump out with a "last" as that would prevent you from checking for other conditions.

    -Kurt

      What this code is doing is building a delayed action dispatch table. Whenever you need to look a variable up in an array, you should consider using a hash. That is exactly their forte.

      my %lookup = ( foo=>\&fooaction, bar=>\&baraction, qux=>\&quxaction ); if (exists $lookup{$var}) { $lookup{$var}->(); } else { #do the default or exception handling here. }

      It cleaner and clearer to read, uses loads less variables, is much easier to maintain and extend and more efficient to boot.

      If brevity is compatible with your mindset, then the if/else can become

      &{ $lookup{$var} || $default_subref }->( parms );

      Examine what is said, not who speaks.
      1) When a distinguished but elderly scientist states that something is possible, he is almost certainly right. When he states that something is impossible, he is very probably wrong.
      2) The only way of discovering the limits of the possible is to venture a little way past them into the impossible
      3) Any sufficiently advanced technology is indistinguishable from magic.
      Arthur C. Clarke.
        Wow! KPeter0314 bows to a Saint

        Looked at this for a couple moments before it really hit me what was happening. I use hashes regularly in my code but never quite like this. Brilliant and slick. I expect a hash lookup is going to be quite efficient. Especially since this turns the problem on its ear and does a hash lookup of the value to a small result set.

        Using the hash reference to return the sub name that you end up executing is my favorite part. (now isn't that a sentence)

        -Kurt

Re: Flag variables
by webfiend (Vicar) on Mar 03, 2003 at 21:04 UTC

    Flags are often the easiest way to say something when you have a small programming vocabulary. In my early Perl code, I used flags all over the place. Now I hardly ever use them, preferring instead to find better ways to ask the question "Is this true?" - at least throwing the messiness off into a subroutine.

    Now, there's nothing wrong with using flag values. They provide a convenient means of saying "do this if that is true". If they are properly named, it's even better: $flag could also be named as $has_foo. Keep your definition of the flag close to where you actually use it, too. That's why I often put flag-reliant code into its own subroutine:

    sub has_phrase { my ($phrase, @wordlist) = @_; my $is_true = 0; foreach my $var (@wordlist) { if ($var eq $phrase) { $is_true = 1; last; } } return $is_true; } # ... called later if (has_phrase("foo", @somearray)) { &someaction }

    As you learn more about syntax and how to express yourself, though, you find these flags getting in the way. Here is a much quicker way to write the same subroutine:

    sub has_phrase { my ($phrase, @wordlist) = @_; foreach my $var (@wordlist) { return 1 if ($var eq $phrase); } return; }

    It could be made even more concise, but this is the limit of my verbose programming style. You get the idea, though. Flags are fine, but they usually represent something you still have to learn about the language.

    And that's not even counting the complete mess you can end up with when you have too many flags. I abandoned one of my early projects eventually, just because it filled up with flags and special circumstances. It ended up getting too hard to read, and not important enough to refactor.


    I just realized that I was using the same sig for nearly three years.

Re: Flag variables
by allolex (Curate) on Mar 03, 2003 at 22:20 UTC

    Maybe wiser heads than mine have posted here, but if you're dying to use flags, you could use constants. So here's an answer to your third question.

    use constant PRINTFLAG => 0; use constant SAVEFLAG => 1; use constant MAILFLAG => 2;

    And then keep track of it with a status/state variable. That at least makes the code easier to read: $state = PRINTFLAG;, instead of just a number. Of course, it's really only approporiate where multiple states come into question.

    --
    Allolex

Re: Flag variables
by oakbox (Chaplain) on Mar 06, 2003 at 08:21 UTC
    First off, thank you for the great discussion and information. Here is a little summary of what I learned here (and aren't summaries what we all want anyway?).

    Using flags in your code (Flag : a variable who's sole purpose in existing is to be checked later on in your script for a particular value.) is just fine with the following caveats:

    1. Try to avoid overuse of flags because it can make your code hard to read and maintain.
    2. Name your flags in a way that indicates what they are measuring.
    3. Never eat yellow snow.
    4. Keep the location where the flag is set and the location where the flag is used close together.

    Again, thank you for the clarifications,

    oakbox

Re: Flag variables
by TomDLux (Vicar) on Mar 05, 2003 at 08:57 UTC
    Flag variables are often a warning of using the wrong program structure.

    On the other hand, I came across some convulted printing code :

    if ( ..A... ) { if ( ..B.. ) { if ( ..C.. ) { if ( ..D.. ) { print ..... } else { print ... } } else { if ( .. D2.. ) { ... you get the idea ...
    It took some effort to reverse engineer the original specs, but I was able to simplify it to:

    my $flag1 = cond1; my $prefix = (( cond2 ) ? "strting1" : "strring2"; ... print "$prefixA$suffixB" if ( condC );
    While things are spread out in the sense that things are detected and saved in flags in one spot, and used later, it avoids dealign with a highly nested conditional ( especially since some possible states were ignored.

    Of course, there are other solutions to the problem I encountered, but that's not relevant tothe topic of flag variables.

Re: Flag variables
by kiat (Vicar) on Mar 04, 2003 at 13:23 UTC
    In the example given, setting the flag is redundant because the &someaction can be called within the loop itself before 'last' is called.

    However there are situations where setting the flag is both useful and perhaps even necessary.

    Say you've just opened a file for both reading and writing. You've a loop to read each line in the file and do something to it if certain conditions are met or print the line back otherwise. You can't exit the loop with 'last' because that would cause the file to be corrupted (some entries at the end may not have been written back). In this case, it may be useful to set a flag to remember the state and do something about it when the loop is finished...

    Did I make sense? Added this:

    Continuing from the above example where you're opening a file for both reading and writing. Say you want to print (on the screen via the browser) an error message because one of the lines contains an error or something. You can't just exit the loop and print the error message because that will interrupt the writing of the file and corrupt it. You need to set a flag to remember the error and display that error when the loop is finished.

      You made sense but, if you can set a flag in the loop, you can also call a subroutine in the same place as you were setting the flag, and the subroutine can do what ever you would have done as a result of the flag being set after the loop, so your back to square one.

      That's not to say that there aren't occasions when a flag is necessary, or even just easier, but most times they can be avoided and the code benefits from their avoidance.

Re: Flag variables
by demerphq (Chancellor) on Nov 27, 2003 at 00:55 UTC
    /^foo$/ and someaction(),last for @somearray;

    ;-)


    ---
    demerphq

      First they ignore you, then they laugh at you, then they fight you, then you win.
      -- Gandhi


Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others making s'mores by the fire in the courtyard of the Monastery: (2)
As of 2024-03-19 07:14 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found