tags:

views:

407

answers:

13

I came across lot of flags while reading someone else code,

if (condition1) 
    var1 = true
else
    var1 = false

then later,

if (var1 == true)
    // do something.

There are lot of flags like this. I eager to know, is using flags very often in code advisable?

+3  A: 

This is pretty subjective, and depends on the rest of the code. "Flags" as you call them have their place.

Nicholas H
+6  A: 

It's advisable if condition1 is something quite complicated - like if (A && (B || C) && !D) or contains a lot of overhead (if (somethingTimeConsumingThatWontChange())) then it makes sense to store that result instead of copy-pasting the code.

If condition1 is just a simple comparison then no, I wouldn't use a flag.

Greg
MatrixFrog
+1  A: 

First of all, this code should read like this:

var1 = condition1;

if( var1 )

// No need to compare *true* to *true* when you're looking for *true*

As for the number of flags, there are more elegant ways of branching your code. For instance , when using javascript you can do stuff like this:

var methodName = someFunctionThatReturnsAString();

// assuming you name the method according to what's returned
myObject[ methodName ]();

instead of

if( someFunctionThatReturnsAString === 'myPreferedMethod' ){
    myObject.myPreferedMethod();
}else{
    myObject.theOtherMethod();
}

If you're using a strongly typed language, polymorphism is your friend. I think the technique is refered to as polymorphic dispatch

Laurent Villeneuve
+10  A: 

This:

if (condition1)
   var1= true;
else
   var1 = false;

Is a classic WTF code.
Instead you should write:

var1 = condition1;

And yes, flags are very useful for making the code be more readable and possibly, faster.

shoosh
+2  A: 

I remember this Replace Temp var with Query method from the refactoring book. I think this refactoring will make the code more readable, but, I agree that it might affect performance when the query method is expensive ... (But, maybe the query method can be put in its own class, and the result can be cached into that class).

Frederik Gheysels
+2  A: 

This is question is a bit generic. The answer depends on what you want to do and with which language you want it to do. Assuming an OO context than there could be better approaches.

If the condition is the result of some object state than the "flag" should propably be a property of the object itself. If it is a condition of the running application and you have a lot of these things it might could be that you should think about a state pattern/state machine.

Norbert Hartl
A: 

If it is readable and does the job then there's nothing wrong with it. Just make use of "has" and "is" prefix to make it more readable:

var $isNewRecord;
var $hasUpdated;

if ($isNewRecord)
{
}

if ($hasUpdated)
{
}
drikoda
A: 

That depends on the condition and how many times it's used. Anyway, refactoring into function (preferably caching the result if condition is slow to calculate) might give you a lot more readable code.

Consider for example this:

def checkCondition():
  import __builtin__ as cached
  try:
      return cached.conditionValue
  except NameError:
      cached.conditionValue = someSlowFunction()
      return cached.conditionValue


As for coding style:

if (condition1)
   var1= true
else
   var1 = false

I hate that kind of code. It should be either simply:

var1 = condition1

or if you want to assure that's result is boolean:

var1 = bool(condition1)


if (var1 == true)

Again. Bad coding style. It's:

if (var1)
vartec
A: 

Bearing in mind that that code could be more readably written as

var1 = condition1

, this assignment has some useful properties if used well. One use case is to name a complicated calculation without breaking it out into a function:

user_is_on_fire = condition_that_holds_when_user_is_on_fire

That allows one to explain what one is using the condition to mean, which is often not obvious from the bare condition.

If evaluating the condition is expensive (or has side effects), it might also be desirable to store the result locally rather than reevaluate the condition.

Some caveats: Badly named flags will tend to make the code less readable. So will flags that are set far from the place where they are used. Also, the fact that one wants to use flags is a code smell suggesting that one should consider breaking the condition out into a function.

D'A

darch
A: 

Call it flags when you work in a pre-OO language. They are useful to parameterize the behaviour of a piece of code.

You'll find the code hard to follow, soon, however. It would be easier reading/changing/maintaining when you abstract away the differences by e.g. providing a reference to the changeable functionality.

In languages where functions are first-class citisens (e.g. Javascript, Haskell, Lisp, ...), this is a breeze.

In OO languages, you can implement some design patterns like Abstract Factory, Strategy/Policy, ...

Too many switches I personally regard as code smell.

xtofl
+1  A: 

Flags are very useful - but give them sensible names, e.g. using "Is" or similar in their names.

For example, compare:

if(Direction)    {/* do something */}
if(PowerSetting) {/* do something else */}

with:

if(DirectionIsUp) {/* do something */}
if(PowerIsOn)     {/* do something else */}
Steve Melnikoff
A: 

What i dont like about flags, is when they are called flags, with no comment whatsoever.

e.g

void foo(...){

     bool flag;

    //begin some weird looking code

    if (something)
       [...]
      flag = true; 
 }

They attempt against code redeability. And the poor guy who has to read it months/years after the original programmer is gone, is going to have some hard time trying to understand what the purposse of it originally was.

However, if the flag variable has a representative name, then i think they are ok, as long as used wisely (see other responses).

Tom
A: 

Yes, that is just silly nonsensical code.

You can simplify all that down to:

if (condition1)
{
  // do something
}
Kon