Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is there such a thing as too many embedded if-statements?

Currently I am working on a bit of code which (I believe) requires quite a few embedded if statements. Is there some standard to how many if statements to embed? Most of my googling has turned up things dealing with excel..don't know why.

If there is a standard, why? Is it for readability or is it to keep code running more smoothly? In my mind, it makes sense that it would be mainly for readability.

An example of my if-structure:

if (!all_fields_are_empty):
    if (id_search() && validId()):
        // do stuff
    else if (name_search):
        if (name_exists):
            if (match < 1):
                // do stuff
        else:
            // do stuff
    else if (name_search_type_2):
        if (exists):
            if (match < 1):
                // do stuff
        else:
            // do stuff
else:
    // you're stupid

I have heard that there's a limit to 2-3 nested for/while loops, but is there some standard for if-statements?

Update: I have some years under my belt now. Please don't use this many if statements. If you need this many, your design is probably bad. Today, I LOVE when I can find an elegant way to do these things with minimal if statements or switch cases. The code ends up cleaner, easier to test, and easier to maintain. Normally.

like image 639
Cody Avatar asked Jun 30 '11 14:06

Cody


3 Answers

As Randy mentioned, the cause of this kind of code is in most cases a poor design of an application. Usually I try to use "processor" classes in your case.

For example, given that there is some generic parameter named "operation" and 30 different operations with different parameters, you could make an interface:

interface OperationProcessor {
   boolean validate(Map<String, Object> parameters);
   boolean process(Map<String, Object> parameters);
}

Then implement lots of processors for each operation you need, for example:

class PrinterProcessor implements OperationProcessor {
    boolean validate(Map<String, Object> parameters) {
       return (parameters.get("outputString") != null);
    }
    boolean process(Map<String, Object> parameters) {
       System.out.println(parameters.get("outputString"));
    }
}

Next step - you register all your processors in some array when application is initialized:

public void init() {
    this.processors = new HashMap<String, OperationProcessor>();
    this.processors.put("print",new PrinterProcessor());
    this.processors.put("name_search", new NameSearchProcessor());
    ....
}

So your main method becomes something like this:

String operation = parameters.get("operation"); //For example it could be 'name_search'
OperationProcessor processor = this.processors.get(operation);
if (processor != null && processor.validate()) { //Such operation is registered, and it validated all parameters as appropriate
   processor.process();
} else {
   System.out.println("You are dumb");
}

Sure, this is just an example, and your project would require a bit different approach, but I guess it could be similiar to what I described.

like image 90
bezmax Avatar answered Sep 21 '22 02:09

bezmax


I don't think there is a limit but i wouldn't recommend embeddeding more the two - it's too hard to read, difficult to debug and hard to unit test. Consider taking a look at a couple great books like Refactoring, Design Patterns, and maybe Clean Code

like image 26
Michael J. Lee Avatar answered Sep 24 '22 02:09

Michael J. Lee


Technically, I am not aware of any limitation to nesting.

It might be an indicator of poor design if you find yourself going very deep.

Some of what you posted looks like it may be better served as a case statement.

I would be concerned with readability, and code maintenance for the next person which really means it will be difficult - even for the first person (you) - to get it all right in the first place.

edit:

You may also consider having a class that is something like SearchableObject(). You could make a base class of this with common functionality, then inherit for ID, Name, etc, and this top level control block would be drastically simplified.

like image 30
Randy Avatar answered Sep 22 '22 02:09

Randy