CODEWITHSUNDEEP
flags
Dhanapal
Dhanapal

Reputation: 14507

Is using flags very often in code advisable?

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?

Upvotes: 8

Views: 3835

Answers (14)

shoosh
shoosh

Reputation: 79021

This:

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

Is a classic badly written code.
Instead you should write:

var1 = condition1;

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

Upvotes: 14

Jose Cifuentes
Jose Cifuentes

Reputation: 596

Here's my take. Code using flags:

...
if (dogIsBarking && smellsBad) {
  cleanupNeeded = true;
}
doOtherStuff();
... many lines later
if (cleanupNeeded) {
  startCleanup();
}
...

Very unclean. The programmer simply happens to code in whatever order his mind tells him to. He just added code at a random place to remind himself that cleanup is needed later on... Why didn't he do this:

...
doOtherStuff();
... many lines later
if (dogIsBarking && smellsBad) {
  startCleanup();
}
...

And, following advise from Robert Martin (Clean Code), can refactor logic into more meaningful method:

...
doSomeStuff();
... many lines later
if (dogTookADump()) {
  startCleanup();
}
...
boolean dogTookADump() {
  return (dogIsBarking && smellsBad);
}

So, I have seen lots and lots of code where simple rules like above could be followed, yet people keep adding complications and flags for no reason! Now, there are legit cases where flags might be needed, but for most cases they are one style that programmers are carrying over from the past.

Upvotes: 0

Kon
Kon

Reputation: 27451

Yes, that is just silly nonsensical code.

You can simplify all that down to:

if (condition1)
{
  // do something
}

Upvotes: 0

Tom
Tom

Reputation: 45174

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).

Upvotes: 0

Steve Melnikoff
Steve Melnikoff

Reputation: 2670

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 */}

Upvotes: 2

vartec
vartec

Reputation: 134711

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)

Upvotes: 0

xtofl
xtofl

Reputation: 41519

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.

Upvotes: 0

darch
darch

Reputation: 4311

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

Upvotes: 0

jedt
jedt

Reputation: 1691

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)
{
}

Upvotes: 1

Norbert Hartl
Norbert Hartl

Reputation: 10851

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.

Upvotes: 2

Frederik Gheysels
Frederik Gheysels

Reputation: 56964

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).

Upvotes: 2

Laurent Villeneuve
Laurent Villeneuve

Reputation: 116

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

Upvotes: 2

Greg
Greg

Reputation: 321824

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.

Upvotes: 7

Nicholas Head
Nicholas Head

Reputation: 3726

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

Upvotes: 4

Related Questions