-6

I have these long if - else/switch statements, really long, and they make the code hard to read. So was just wondering is there a way to get rid of them? Maybe something like using a class for each condition and then using chain of responsibility pattern ?

What kind of approach would you suggest ?

Here is a sample code:

    if (  cc == LIB_DEFINED_CONSTANT_1  )
    {

        response = "-1";

        errorWindow->setText ( "The operation timed out.\nPlease try again later." );
        errorWindow->show (  );

    }   

    else if (  cc == LIB_DEFINED_CONSTANT_2  )
    {

        response = "-1";

        errorWindow->setText ( "Couldn't conect to the server.\nPlease try again later." );
        errorWindow->show (  );

    }

    else if (  cc == LIB_DEFINED_CONSTANT_3  )
    {

        response = "-1";

        errorWindow->setText ( "Access is denied.\nPlease contact our support team." );
        errorWindow->show (  );

    }

    else if (  cc == LIB_DEFINED_CONSTANT_4  )
    {

        response = "-1";

        errorWindow->setText ( "Credentials and varified\nPlease contact our support team." );
        errorWindow->show (  );

    }
    else if ....

As you can see, most of the code in the conditional tag is some what similar, except for setting text for errorWindow.

EDIT : It would be nice if people can leave comment on why they down voted.

StudentX
  • 2,243
  • 6
  • 35
  • 67
  • 7
    It depends on what problem they solve, but it's probably something that can be resolved with an array, a map or a state machine. – Morwenn Jun 27 '13 at 09:25
  • 2
    Who knows... without seeing the code or at least a more comprehensive explanation as to what the conditions being tested are and what code is being executed in each controlled block. – CB Bailey Jun 27 '13 at 09:27
  • 4
    Without code, I have no idea. Put a minimal sample in your question, with say 2 or 3 of your conditions. Whithout more context you won't get much of an answer. – rectummelancolique Jun 27 '13 at 09:27
  • 1
    This might be a useful starting point http://stackoverflow.com/questions/505454/large-switch-statements-bad-oop?rq=1 – Marcello Romani Jun 27 '13 at 09:34
  • Now that we have the got, it's gotta be an array. – Morwenn Jun 27 '13 at 09:38
  • 2
    In your example, you are duplicating _everything_ except the contents of a string. Start by eliminating duplication, i.e. factoring out lines which are exactly the same to a single place. You'll be left with a table of error codes and string descriptions, and you'll be able to tackle _that_ specific problem. – Daniel Daranas Jun 27 '13 at 09:47
  • 2
    I think this question would be better placed at http://codereview.stackexchange.com , because the code is working in the way it should (as far as I can see from your question), and you're asking for suggestions for improvement. I think @DanielDaranas has given the most useful hint so far: store the error codes in one array (preferably `std::vector`), and the corresponding error messages in another array (`std::vector`), or create a `struct` containing error code and message, and put all of them into an array. The details are left as an exercise to the reader ;) – Piotr99 Jun 27 '13 at 11:58

1 Answers1

2

In general, you should try to split your code into small functions/methods, and each of them should do exactly one thing. If you have a very long (several pages) if/else block, you should probably refactor your code.

I think that code should read like this:

if (shouldIdoThing1()) 
{
  doThingOne(withThis, andThis, andThat);
}
else if (shouldIdoThing2())
{
  doTheSecondThing(withThisOnly);
} 
else
{
  doTheOtherThing(withSomethingElseEntirelyPerhaps);
}

and, if possible, I try to make my code look like that. In large applications scrolling through several pages just to check what is done in else is a real pain in ... one's head.

Dariusz
  • 21,561
  • 9
  • 74
  • 114
  • I think this isn't addressing the question, as it specifically asked to get rid of the if/else statements. – Piotr99 Jun 27 '13 at 12:01
  • @Piotr99 he wants to get rid of "really long" statements which "make the code hard to read". My answer addresses these issues. – Dariusz Jun 27 '13 at 12:58