Getting rid of / Minimizing a lot of if statements?

  • Thread starter Thread starter Guest
  • Start date Start date
G

Guest

I have some code that works fine, except for the fact that it has a lot of
embedded if statements. I don't think that this would be practice code, but I
wanted to tap into the knowledge of this group, and see what is the best way
to handle it. Basically, the if statements are used to check several
conditions of whether something is activated etc., and if it is, it outputs a
small piece of text and redirects the user to a new page. For each of the
conditions, there are 20 if statements. In the below, I am putting 1 piece,
but there are 20 IDENTICAL other lines to these, for each of the the ten
conditions. The first checks whether the condition is active, if the user is
an admin, and displays some text.
if (AdminUser())
{
if (_enableCondition1 == true)
{
output.Write(@"<img border=""0"" src=""foo""
align=""middle"">);
output.Write("<b>");
output.Write("foo" + "<br>");
}
else
{
output.Write("Foo Not Active" + "<br>");
}
}
.......

(then there are 20 more of these!)

then the other type is handled if it is found active:

if (_enableCondition1 == true)
{
if (foo1 == foo2)
{
fooMethod
}
}

Also 20 of these.

This seems to be inefficient to me. Is there a better way of handling it?
 
XSler said:
I have some code that works fine, except for the fact that it has a lot of
embedded if statements. I don't think that this would be practice code, but I
wanted to tap into the knowledge of this group, and see what is the best way
to handle it. Basically, the if statements are used to check several
conditions of whether something is activated etc., and if it is, it outputs a
small piece of text and redirects the user to a new page. For each of the
conditions, there are 20 if statements. In the below, I am putting 1 piece,
but there are 20 IDENTICAL other lines to these, for each of the the ten
conditions. The first checks whether the condition is active, if the user is
an admin, and displays some text.
if (AdminUser())
{
if (_enableCondition1 == true)
Click to expand...

First thing: you never need to compare conditional expressions with
'true', you can just use:

if (_enableCondition1)
{
output.Write(@"<img border=""0"" src=""foo""
align=""middle"">);
output.Write("<b>");
output.Write("foo" + "<br>");
}
else
{
output.Write("Foo Not Active" + "<br>");
}
}
Click to expand...

The basic pattern here is to choose to do one thing or another, based on
the truth of a third thing. The problem, the ugliness, is that it's
code. Consider how things would change if these choices were data:

---8<---
using System;
using System.Collections.Generic;

class App
{
delegate bool Predicate();
delegate void Action();

class ConditionalAction
{
Predicate _pred;
Action _ifTrue;
Action _ifFalse;

public ConditionalAction(Predicate pred,
Action ifTrue, Action ifFalse)
{
_pred = pred;
_ifTrue = ifTrue;
_ifFalse = ifFalse;
}

public void Execute(/* args maybe */)
{
if (_pred(/* args maybe */))
_ifTrue(/* args maybe */);
else
_ifFalse(/* args maybe */);
}
}

static void Main()
{
List<Action> actions = new List<Action>();
actions.Add(new ConditionalAction(
delegate { return true; },
delegate { Console.WriteLine("True"); },
delegate { Console.WriteLine("False"); })
.Execute);

foreach (Action action in actions)
action(/* args maybe */);
}
}
--->8---

This makes sense if you can find a way to fill the 'actions' list from
somewhere else, such as a configuration file, or otherwise
*declaratively* specify it.

That might satisfy your thirst for more neatness. However, beware! It
may be too much complexity, more complexity than it's worth.

-- Barry
 
You might consider the "Factory Design Pattern"


public interface IDoSomeWork
void DoWork()

public ConcreteClass1 : IDoSomeWork
void DoWork

public ConcreteClass2 : IDoSomeWork
void DoWork


Then you create a "Factory" . A factory is basically a class, which creates
other classes.


public class WorkFactory
public static GetAWorker( bool someCondition )
{
if (someCondition)
{return new ConcreteClass1() );
else
{ return new ConcreteClass2() ) ;
}



bool sc = true;
IDoSomeWork idsw = WorkFactory.GetAWorker( sc ) ;
idsw.DoWork();

--------
Real life:
Imagine you need to pay employees. You have salaried and hourly employees.
hourly employees get their hours * wage. salaried people get some fraction
of their yearly income.

instead of this (your issue)

if (isSalaried)
{
//logic to pay a salaried employee
}
else
{
//logic to pay an hourly employee
}


-- BOO !! Hard to maintain code.
Here is the alternative.




public interface IPayEmployee
void PayOut()

public PayHourlyEmployee : IPayEmployee
void PayOut ( //take hours * wage and pay employee);

public PaySalariedEmployee : IPayEmployee
void PayOut ( //find yearly salary and divide by 52 (aka , a fixed rate)
;


public class PayFactory
public static GetAPayer( bool isSalaried )
{
if (isSalaried )
{return new PaySalariedEmployee () );
else
{ return new PayHourlyEmployee () ) ;
}


bool isSalaried = true;
IPayEmployee ipe = PayFactory.GetAPayer( isSalaried ) ;
ipe.PayOut();


And....when you need a hybrid (an employee who gets paid salaried, but if
goes over 60 hours, gets an hourly rate for example),
you just add a new class, and alter the Factory a little.


www.dofactory.com and look for factory pattern under "free design patterns".



see also:
12/1/2005
Understanding the Simple Factory Pattern
http://sholliday.spaces.live.com/blog/
 
Hello,

In order to minimze the number of IFs multiple IF can be used togather
using AND or OR conditions wherever possible.

if (AdminUser() && _enableCondition1 == true)
{
{
output.Write(@"<img border=""0"" src=""foo""
align=""middle"">);
output.Write("<b>");
output.Write("foo" + "<br>");
}
else
{
output.Write("Foo Not Active" + "<br>");
}
}

You can also exclude the exceptions that come in else in order to
reduce ELSEs in the code using RETURN statments. like

if ( ! AdminUser() OR _enableCondition1 == false)
return;
output.Write(@"<img border=""0"" src=""foo"" align=""middle"">);
output.Write("<b>");
output.Write("foo" + "<br>");
}

HTH.
 
Back
Top