0
\$\begingroup\$

I've got a part of my application which deals with tracking the dates a particular item is worked on. It is a single aspnet page, which displays controls to either start or stop tracking, depending on the state of the work item.

There are up to three controls on the page. A calendar to enter the date that work was stopped/started. A dropdown to select a reason the work item was stopped. A text area to enter a comment.

Because a similar set of controls will be created on both possible work item states (either started or stopped), I've decided to create a single set of controls, and modify them depending on the state of the work item.

The markup I use for the controls is below:

<div id="DatePickerContainer">
    <span id="DatePickerLabel" runat="server"></span>
    <asp:Calendar id="DatePickerControl" runat="server"></asp:Calendar>
</div>

<div id="DropdownContainer" runat="server">
    <span id="DropdownLabel" runat="server"></span>
    <asp:DropDownList id="DropdownControl" runat="server"></asp:DropDownList>
</div>

<div id="TextReasonContainer">
    <span id="TextReasonLabel" runat="server"></span>
    <textarea id="TextReasonControl"></textarea>
</div>

And the code behind to initialise:

void Page_Load(Object sender, EventArgs e)
{
    if (this.WorkItemIsStopped())
    {
        this.SetupStartControls();
    } 
    else 
    {
        this.SetupStopControls();
    }
}

void SetupStartControls()
{
    this.DatePickerLabel.InnerText = "Start date";

    this.DropdownContainer.Visible = false;

    this.TextReasonLabel.InnerText = "Start comment";
}

void SetupStopControls()
{
    this.DatePickerLabel.InnerText = "Stop date";

    this.DropdownLabel.InnerText = "Stop Reason";
    this.DropdownControl.DataSource = this.CreateDropdownDataSource();

    this.TextReasonLabel.InnerText = "Stop comment";
}

Is this a reasonable approach? The alternative would be to create two similar but separate sets of controls in the markup, and simply hide the set which are not relevant to the current state of the work item.

\$\endgroup\$
1
\$\begingroup\$

The alternative would be to create two similar but separate sets of controls in the markup

The terms similar and separate are confusing when considering class design. So allow me to think this through...


A calendar, dropDown, and textBox. - parts of a single class

From the given information both start and stop have these. And the only difference is the labels. So you can have one class, instantiating 2 objects, one for "stopping" and one for "starting".

public class StartStopper {
   //DatePicker control
   //DropDown control
   // TextBox control

   public StartStopper (string DatePickerLableText, string dropdownText, string reasonLabelText) { //assign the strings } 
}

------------------------------------------------------------------

The alternative would be to create two similar but separate sets of controls in the markup, and simply hide the set which are not relevant to the current state of the work item.

Here is an alternative:

The Design Paradigm

  • All the layering that's about to be done is all about the single responsibility principle applied well.

  • Classes start small and focused. DropBox and Textbox have distinct behavior and business rules between themselves. So the single responsibility of a StartStopBox class is to provide the all the behavior and features of their intimate, private, interaction - and nothing else.

  • DropDown, TextBox interaction and rules are what they are. It does not matter if it is for "stopping" or "starting". Either way, for example, "select the dropdown, the textbox must have an entry."

  • Aggregate classes into a StartStopCalendar class - it's single responsibility is to make the component classes interact, giving us more complex behavior. Now we can tell when or if the calendar requires a value.

  • If any component needs more behavior for it's share of the interaction, fine. But that behavior is not StartStopCalendars responsibility. Remember, it's single responsibility is to make that new behavior interact with other components.

  • Net result is classes are more re-usable - it carries all it's own baggage. There is less coupling - Change what "I am valid" means and the clients don't know! They just call the same method as always.


Business Rules affecting Design

All 3 controls always work interactively

If business rules apply to all 3 controls together then let's keep them in one class. The calender has rules relating to the dropdown, and separate rules relating to the text box. And the dropdown and textbox have their relation rules too.

In this case, one class to rule them all. However we assume these rules are the same; it doesn't matter if we're going to use is for "stop" or "start". Below we'll see how we get that context through class composition.


DropDown and textbox work together. This focused interaction is independent of the calendar

The dropDown and Textbox should be one class, and the calendar another. Further the calendar requires a StartStopBox object so we aggregate them:

public class StartStopBox {
    // "start" or "stop" work the same, only the labels have changed.
    public StartSTopBox ( string dropDownLabelText, string TextBoxLabelText) {}


    // This is ugly, and not so single responsibility.
    // The client is forced to evaluate the returned string
    // to decide if the object is valid.
    // But it's expedient and we'll see this get encapsulated later.
    public string Validate() {
        string errorMessage = string.Empty;

        if (! dropdown.IsEmpty() && ! TextBox.IsEmpty() {
             errorMessage = "\nDropDown and TextBox must both have enteries";
        }
        return errorMessage;
    }
}

public class StopStartCalendar {
     public StartSTopCalendar (StartStopBox aStarterOrStopper) {  }

      public string Validate () {
          string errorMessage = string.Empty;
          bool startStopperIsValid = false;

          errorMessage += aStarterOrStopper.Validate();
          startSTopperIsValid = string.IsNullOrEmpty(errorMessage) ? true : false;

          if (string.IsNullOrEmpty (theCalendar.DateSelected() && startStopperIsValid) // or however it's done.
          { errorMessage += "\nA date must be selected"; }

          return errorMessage;
      }
}

We have business rules for a "start object" interacting with a "stop object"

For example "cannot stop earlier than we started"

public class Interval {
     public Interval ( StopStartCalendar starter, StopStartCalendar stopper)
     {
         public bool isValid() {
             return string.IsNullOrEmpty(Validate()) ? true : false
         }

         public string Validate() {
             string errorMessage = string.Empty;
             errorMessage += "Starter object errors: \n\n" + starter.Validate();
             errorMessage += "Stopper object errors: \n\n" + stopper.Validate();

             if (starter.Date > stopper.Date ) {
                errorMessage += "\n\nStop Date cannot be Earlier than Start Date";
             }
             return errorMessage;
         }
     }

Out of the Trees and into the Forest

... It is a single aspnet page, which displays controls to either start or stop tracking, depending on the state of the work item.

public ProjectState { Undefined, Planning, Started, Ended }

public class Project {
    protected ProjectState currentState = Undefined;

    public Project ( Interval projectDates ) { }
    ...

Wait! Now I want to be able to ask the StartStopCalendar if it is in it's proper state, and single responsibility principle guides us in making changes.

public class StartStopBox {
    // Make it simpler, less error prone for the client.
    public bool IsValidState() {
        return string.IsNullOrEmpty(this.Validate()) ? true : false;
    }
} 

public class StartStopCalendar {
    // ditto
    public bool IsValidState() {
        return string.IsNullOrEmpty(this.Validate()) ? true : false; 
    }
} 

public class Interval{
     . . .

     public bool IsStartState() {
         return ( starter.IsValidState() && !ender.IsValidState() );
     }

     public bool IsEndState() {
         return ( !starter.IsValidState() && ender.IsValidState() );
     }   
}

public class Project {
    protected ProjectState currentState = Undefined;

    public void StartProject () {
        if (startStuff.IsStartState()) {
            currentState = ProjectState.Planning;
            DoStartingStuff(true);
        }else
           currentState = ProjectState.Undefined;
           DoStartingStuff(false);
        }
    }
}

ALMOST THERE

public class ProjetManager {
   protected Project me { get; set; }

   public ProjectStatus WhereAreYouOnThatProject () {
        return me.currentState;
   }

   public string ArentYouDoneYet() {
       return me.ProjectDates.IsEndState() ? "Yes" : "My dog ate my Homework";
   }

}

// THE CLIENT

public bob = new ProjectManager ( bobsProject); 

bob.WhereAreYouOnThatProject();
bob.ArentYouDoneYet();

bob.YoureFired();

" NotImplementedException was thrown.  Stack trace ....."
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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