views:

271

answers:

5

I want to refactor some code.

Basically the code I want to refactor is a Form (using System.Windows.Forms;)

The way it is setup now, depending on which radio button you selected it shows a different layout for the window: different labels, buttons, etc. Not always a big difference, but different. This is a lot of conditional statement junk all over the place. I wanted instead to refactor it with the State pattern. There are three main states.

I'm not sure the best way to do this. Right now the IState interface has a DoAction() method which does some action particular to the unique State, and a DrawForm() method which re-draws the form based on the current State... However, to do the DrawForm(), the State classes need to be able to access the Form's member variables. That's what threw me for a loop. I didn't really want to expose them.

Is there a better way to do this?

+3  A: 

You could make your state classes nested in your form. They will then be able to acces fields of form without having to expose them.

Ben Lings
Ahhh, great point. Thanks.
Alex Baranosky
This is pretty much exactly what I needed to feel good about the way I was setting it up. Somehow I totally forgot about nested classes.
Alex Baranosky
A: 

Why don't you place a public method inside the form called Draw that takes in an IState interface, or even just an enumeration value and designs the form according to whatever state it is in i.e.

public partial class MyForm : Form
{
    public MyForm()
    {
        InitializeComponent();
    }

    public void Draw(IState state)
    {
        switch (state.Value)
        {
            case 1:
               DrawState1();
               break;
            case 2:
               DrawState2(); 
               break;
        }
    }

    private void DrawState1()
    {
       // design for state 1
    }

    private void DrawState2()
    {
       // design for state 2
    }

    ....
}

Then the usage:

public void OnSomeControlClick(object sender, EventArgs e)
{
    IState state = new State();
    // check which option has been selected
    if (Option1.Checked)
    {
        state.Value = 1;
    }
    else if (Option2.Checked)
    {
        state.Value = 2;
    }

    // if this method is inside the form itself
    this.Draw(state);
}

To reduce the amount of code per method you could just design each form state at design-time on separate panels then its just a case of hiding/showing each panel.

James
I chose to put the different Draw() methods in the state itself. This way I eliminate the switch statements.
Alex Baranosky
How do you work out what state is to be drawn?
James
A: 

I don't know if this helps, but why not have something like a strategy pattern for drawing the form?

So something along the lines of

interface IDrawStrategy
{ 
   void Draw(FormType form);
}

And then when you pick which state the form is in, you can assign the form's draw strategy. Yes, you will still need to give the draw method access to the Forms variables to allow it to position them, but it at least allows you to make it easier to add different looks based on the states.

Furis
I have a DrawForm() method for each State the form can be in. It's pretty similar to what you are saying.
Alex Baranosky
A: 

In wanting to extract the "state" of the Form, you are describing a pattern better known as a "Presentation Model". You will find some good information and tips on how you generally want the Form and State to interact by reading this article by Martin Fowler. In your case, your Form is the "View" and your State is the Presentation Model. As always, Fowler does a great job answering this question and more.

Chris Melinn
I will take a look at that, thank you.
Alex Baranosky
+1  A: 

If your primary concern is keeping form stuff encapsulated, and you want to keep the state machine outside, I had a similar question a while back, you could check out the answer here - basically it entails creating a private inner 'action' class which DOES have access to form methods, then passing this to the state machine so that it can use the action class to invoke form methods indirectly.

Benjol