Tell me more ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.
up vote 0 down vote favorite

The project is full of such code and the function is really long and hard to understand.. How to refactor? 

int get_greate_structure(GreateStructure* st)
{
    switch (st->type)
    {
     case 1:
           do_some_business_and_fill_part_of_the_st();
           break;
     case 2:
           do_other_things_and_fill_part_of_the_st();
           break;
     case 3:
           break;
            ...

     case 1000:
           ...
    }
}

void fn()
{
     GreateStructure *st = AllocateGreateStructure();
     st->do_case_1 = true;
     get_greate_structure(st);
     // we did the case 1 and get some data we need..

}
share|improve this question
using some speaking constants for the cases, would be my first idea. – K.. Jan 31 at 15:33
1  
@K.. constants? Surely you mean enums. – MichaelT Jan 31 at 15:37
Or use a dictionary to store the link between a label and an action. But probably this entire structure needs a large overhaul. – Paul Hiemstra Jan 31 at 15:38
@MichaelT yes, or this :) – K.. Jan 31 at 15:47
the switch case is too large and each case has no specific meaning. try breaking them up and see if you could give names instead of meaningless numbers. then again, if get_greate_structure is already working then you shouldn't have to look at it. – resting Jan 31 at 15:49
show 3 more comments

closed as off topic by Glenn Nelson, Robert Harvey, Walter, gnat, MichaelT Jan 31 at 20:47

Questions on Programmers Stack Exchange are expected to relate to software development within the scope defined in the FAQ. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about closed questions here.

4 Answers

up vote 0 down vote

In general, if you have code with switch statements, you should try to see if you can solve this problem using polymorphism. @Gary Rowe's and @SpaceTrucker's suggestions of using a state machine or a command pattern are merely specific examples of using polymorphism.

share|improve this answer
up vote 0 down vote

Have a look at the command pattern. There are several SO questions on this that mention a analogous problem. See here and here.

Basically you define an interface with a method

doSomeActionAndFillSt(GreateStructure* st)

Then you define a map that maps a st->type instance to an instance of the above interface.

share|improve this answer
up vote 7 down vote

Turn it into a state machine

This will allow you to decompose this great beast of operations into a collection of small highly targeted objects/modules that just do one simple thing. This adheres to both DRY and KISS principles.

share|improve this answer
up vote 5 down vote

It is probably a good idea to cut up your large data structure into smaller, more cohesive objects with relations between those objects. In this way, there is no need for a big, centralized, means of filling the big object. Without more details, it is hard to give specific answers.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.