oakbox has asked for the wisdom of the Perl Monks concerning the following question:
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.
|
---|
Replies are listed 'Best First'. | |
---|---|
Re: Flag variables
by zby (Vicar) on Mar 03, 2003 at 10:53 UTC | |
| [reply] [Watch: Dir/Any] [d/l] |
Re: Flag variables
by robartes (Priest) on Mar 03, 2003 at 10:57 UTC | |
More learned minds than mine will undoubtedly come up with other reasons why flag variables are often "bad". CU | [reply] [Watch: Dir/Any] [d/l] [select] |
Re: Flag variables
by Aristotle (Chancellor) on Mar 03, 2003 at 13:26 UTC | |
[reply] [Watch: Dir/Any] | |
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:
This keeps the trigger condition and action close together, and doesn't do unnecessary checks once a match is found. | [reply] [Watch: Dir/Any] [d/l] [select] |
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. This code uses not one but THREE flags, transforming a would-be OOP module into a peculiar example of spaghetti code.
| [reply] [Watch: Dir/Any] [d/l] [select] |
Re: Flag variables
by Anonymous Monk on Mar 03, 2003 at 12:45 UTC | |
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. :-) | [reply] [Watch: Dir/Any] |
Re: Flag variables
by blakem (Monsignor) on Mar 03, 2003 at 10:58 UTC | |
Or even:
-Blake | [reply] [Watch: Dir/Any] [d/l] [select] |
by jsegal (Friar) on Mar 03, 2003 at 15:34 UTC | |
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 | [reply] [Watch: Dir/Any] [d/l] |
by adrianh (Chancellor) on Mar 07, 2003 at 11:05 UTC | |
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. | [reply] [Watch: Dir/Any] [d/l] |
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:
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? | [reply] [Watch: Dir/Any] [d/l] [select] |
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:
Update: sorry for the duplicate reply - zby beat me to it | [reply] [Watch: Dir/Any] [d/l] |
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:
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' | [reply] [Watch: Dir/Any] [d/l] [select] |
by IlyaM (Parson) on Mar 07, 2003 at 11:37 UTC | |
or more Perl-ish
-- | [reply] [Watch: Dir/Any] [d/l] [select] |
Re: Flag variables
by KPeter0314 (Deacon) on Mar 03, 2003 at 15:35 UTC | |
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. Or, would there be a better way than expanding the code to this if the @somearray were large: Note: you wouldn't want to jump out with a "last" as that would prevent you from checking for other conditions.
-Kurt | [reply] [Watch: Dir/Any] [d/l] [select] |
by BrowserUk (Patriarch) on Mar 03, 2003 at 16:37 UTC | |
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.
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
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. | [reply] [Watch: Dir/Any] [d/l] [select] |
by KPeter0314 (Deacon) on Mar 03, 2003 at 19:58 UTC | |
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 | [reply] [Watch: Dir/Any] |
by BrowserUk (Patriarch) on Mar 03, 2003 at 21:05 UTC | |
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:
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:
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. | [reply] [Watch: Dir/Any] [d/l] [select] |
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.
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.
-- | [reply] [Watch: Dir/Any] [d/l] [select] |
Re: Flag variables
by oakbox (Chaplain) on Mar 06, 2003 at 08:21 UTC | |
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: Again, thank you for the clarifications, | [reply] [Watch: Dir/Any] |
Re: Flag variables
by TomDLux (Vicar) on Mar 05, 2003 at 08:57 UTC | |
On the other hand, I came across some convulted printing code :
It took some effort to reverse engineer the original specs, but I was able to simplify it to:
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. | [reply] [Watch: Dir/Any] [d/l] [select] |
Re: Flag variables
by kiat (Vicar) on Mar 04, 2003 at 13:23 UTC | |
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. | [reply] [Watch: Dir/Any] |
by Anonymous Monk on Mar 04, 2003 at 14:19 UTC | |
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. | [reply] [Watch: Dir/Any] |
Re: Flag variables
by demerphq (Chancellor) on Nov 27, 2003 at 00:55 UTC | |
;-)
---
demerphq First they ignore you, then they laugh at you, then they fight you, then you win.
| [reply] [Watch: Dir/Any] [d/l] |