Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Good day,

I need criticism/suggestions on a suitable way of controlling many states of display in a GUI like method, "GUIMethod", below.

Edited: Three scripts in question A,B,C. C is enabled. Once we click a button (produced by C) we disable C and enable B. A and B toggle each other based on clicking events.

/*
 *Script 1
 */
public class Script_1 {

    private enum ScriptOneStates {
        A, B, C, D, E, //...
    }

    private ScriptOneStates states; 

    private GUIMethod() {
        switch (states) {
        case ScriptOneStates.A:
            Method1_A();
            Method2_A();    
            break;
        case ScriptOneStates.B:
            Method1_B();
            Method2_B();
            break;
        }
    }

    private void Method1_B() {
        if (InputManager.Button("Forward")) {
            GetComponent<Script_1>.isEnabled = true;
            GetComponent<Script_2>.isEnabled = false;
        }
    }
 }
/*
 *Script 2
 */
public class Script_2 {

    private enum ScriptTwoStates {
        A, B, C, D, E, //...
    }

    private ScriptTwoStates states; 

    private GUIMethod() {
        switch (states) {
        case ScriptTwoStates.A:
            Method1_A();
            Method2_A();    
            break;
        case ScriptOneStates.B:
            Method1_B();
            Method2_B();
            break;
        }
    }

    private void Method1_B() {
        if (InputManager.Button("Back")) {
            GetComponent<Script_2>.isEnabled = false;
            GetComponent<Script_1>.isEnabled = true;
        }
    }
 }

The type 'GetComponent' is a global reference to access scripting components. Given that this is okay, I would like to know if this is acceptable.

share|improve this question

migrated from stackoverflow.com Jan 17 '13 at 2:28

This question came from our site for professional and enthusiast programmers.

    
I'm not sure I can really comment on this solution here. The always annoying answer of "it depends" applies here. Could you please go one step back and instead of showing the code, lay out your requirements and what the GUI should accomplish? –  Ameen Jan 16 '13 at 22:10
    
@avidprogrammer, I find your example too abstract to efficiently answer the question. I would want to know your specific application at hand; that is just how my brain works. –  Leonid Jan 19 '13 at 18:58

2 Answers 2

In a case like this, you could consider writing classes to implement the functions you want your GUI to do. One per enum member, with a base class. Then you could do:

    GuiStateObject myState;

in your GUI class; switch state any time you want with

    myState=new EasyGuiState();

and do the GUI work by calling

    myState.WillDoAnythingIWantUsingOverriddenBaseFunctions();

You can extend the idea if there are things you dont want to do if in a state and do want to do if in another:

    if (myState.WillDoThisThing)
           myState.DoIt();
share|improve this answer
2  
sorry for formatting :(, on phone –  canahari Jan 16 '13 at 23:38
4  
+1 for answering a CR question on your phone!! –  Mat's Mug Jan 18 at 1:14

Well... first of all you could insert your

public static InterfaceState interfaceState;

In your main form class or in your entry point class so you don't need to handle it with a struct. I don't really understand the rest of the code and I don't really know what are those states for... from what I can understand, however, the GUI can have one and only one state at once, so distributing your switch into different classes is not worth the effort unless you are doing it for code maintainability reasons. So... put together all your various GUIMethod() inside a single GUIMethod() which handles every state if the pieces of code that follow the various cases are well organized into small functions. You have to make your code easy to maintain (avoiding excessive complexity) and also easy to debug/understand.

public sealed class ApplicationForm : Form
{
    private enum InterfaceState
    {
        ENUMS,
        SEEM,
        EASY,
        TO,
        WRITE,
        TODAY
    }

    private InterfaceState m_State;

    public ApplicationForm()
    {
        m_State = InterfaceState.ENUMS;
    }

    private void ActionByState()
    {
        switch (m_State)
        {
            case InterfaceState.ENUMS:
                ENUMSMethod1();
                ENUMSMethod2();
                m_State = InterfaceState.SEEM; // Next Time ActionByState() is called you do SEEM...
                break;

            case InterfaceState.SEEM:
                SEEMMethod();
                goto case InterfaceState.EASY; // When SEEM state is processed, you always do EASY...

            case InterfaceState.EASY:
                EASYMethod();
                break;

            case InterfaceState.TO:
                TOMethod1();
                TOMethod2();
                TOMethod3();
                break;

            case InterfaceState.WRITE:
                WRITEMethod();
                break;

            default:
                TODAYMethod();
                break;
        }
    }
}
share|improve this answer
    
Thanks for the response. –  avidprogrammer Jan 16 '13 at 22:20
    
You're welcome! –  Zarathos Jan 16 '13 at 22:21
    
Edited; perhaps it is more understanable. Can I repost w/o being banned? –  avidprogrammer Jan 17 '13 at 2:43
    
I don't think that distributing a switch across classes is EVER a good idea. It screams inheritance like what canahari was suggesting. Infact if the OP could make a base abstract class, he could easily instantiate teh appropriate child class and just make the same call no matter what. And if done correctly it could even remove all the switch statements. I'm seeing the use of custom events that return the correct child class... but that is just me. –  Robert Snyder Feb 16 '13 at 17:04

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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