Page_Load-routine

Question

I have the following .net-page. I hope it is ok that it is only an example, describing my problem(s) pretty well.

<form id="form1" runat="server">
<div>    
    <asp:TextBox runat="server" ID="textBox1"></asp:TextBox>
    <asp:TextBox runat="server" ID="textBox2"></asp:TextBox>

    <asp:Calendar runat="server" ID="calendar1"></asp:Calendar>

</div>
</form>

public partial class WebForm1 : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        if (!Page.IsPostBack)
        {
            textBox1.Text = "Hello";
            textBox2.Text = "User";

            DateTime yesterday = DateTime.Now.Date.AddDays(-1);
            calendar1.SelectedDate = yesterday;
        }
    }        
}

The Page_Load-routine fills two textboxes with some text and sets the selected date of a calendar control to yesterday.

My questions are:

1. Do I need to or is it recommended to outsource any code into separate routines? I could f.ex. outsource the "calendar part" of the code into a routine SetCalendar, but is it best practice? (It is clear that I would outsource the code if needed on several places in the page class, but this is not the case here.)
2. Do you think the routine conforms with the Single Responsibility Principle? (A routine should have one and only one responsibility) Is the principle of any importance in a module of this type (event)?
3. I read that a routine should be as generic as possible. If I had to outsource the "calendar part", would it be good to hand over the date as a parameter, even if I never set the calendar to any other date than yesterday?

Answer 1

• I don't see any reason to refactor that code, at least not in its current state.
• Yes, the routine has a single responsibility at this stage, namely to initialise the fields in the form. If you add another responsibility to the method, you can put the initialisation code in a separate method.
• If you would make a general method to set the calendar date, it would take the calendar reference and the date, so you would just make a method to encapsulate the property setter, which is completely pointless.

You could refactor the code if you choose to see the responsibilities based on separate form fields rather than the form as a whole. However, you should then consider a more object oriented approach rather than just moving the code into separate methods. You can make classes that have the responsibility for initialising, validating and reading a type of form field.

Note: The property calls DateTime.Now.Date already exists as the property DateTime.Today.

Answer 2

I would do something like this:

public partial class WebForm1 : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        if (Page.IsPostBack) return;

        InitGreetingMessage();
        InitCalendar();
    }        

    private void InitGreetingMessage()
    {
       textBox1.Text = "Hello";
       textBox2.Text = "User";
    }

    private void InitCalendar()
    {
       var yesterday = DateTime.Today.AddDays(-1);
       calendar1.SelectedDate = yesterday;
    }
}

Answer 3

My own approach looks like this and is based on the answers of the users so far.

public class FormInitializer
{
    public void SetCalendarDate(Calendar c, DateTime date)
    {
        c.SelectedDate = date;
    }

    public void SetTextBoxText(TextBox tb, string text)
    {
        Check.NotNullOrEmpty(text, String.Format("No text provided for TextBox with ID {0}", tb.ID.ToString()));
        tb.Text = text;
    }
}

public partial class WebForm2 : System.Web.UI.Page
{
    FormInitializer formInitializer = new FormInitializer();

    protected void Page_Load(object sender, EventArgs e)
    {          
        if (!Page.IsPostBack)
        {
            formInitializer.SetTextBoxText(textBox1, "Hello");
            formInitializer.SetTextBoxText(textBox2, "User");
            formInitializer.SetCalendarDate(calendar1, DateTime.Today.AddDays(-1));
        }
    }        
}

I like some things about this approach.

• It improves readability -> it is much clearer to the reader what the module Page_Load is assigned to do, because I can influence the naming of modules;
• I have fine-grained control about business rules; f.ex. if the user wants to insert text, he can not do so with a String.Empty;
• as the modules are put into a specialized class, this improves code reusability

It is true that the SetCalendarDate module only sets one property, but I can live with it because of the other advantages I get and for the sake of the unity of the concept.