Composite Controls made easy

Scott Gu has an interesting article in his blog, pointing to another article in MSDN, about creating Composite Controls.

Its a great read, a must read, even.

I just have one problem with it. Near the beginning of the article there is some sample code as follows:

public class LabelTextBox : WebControl, INamingContainer
{
   public string Text {
      get {
         object o = ViewState["Text"];
         if (o == null)
              return String.Empty;
         return (string) o;
      }
      set { ViewState["Text"] = value; }
   }
   public string Title {
      get {
         object o = ViewState["Title"];
         if (o == null)
              return String.Empty;
         return (string) o;
      }
      set { ViewState["Title"] = value; }
   }
   protected override void CreateChildControls()
   {
      Controls.Clear();
      CreateControlHierarchy();
      ClearChildViewState();
   }
   protected virtual void CreateControlHierarchy()
   {
       TextBox t = new TextBox();
       Label l = new Label();
       t.Text = Text;
       l.Text = Title;
       Controls.Add(l);
       Controls.Add(t);
   }
}

This is supposed to be an example of how you can use composite controls to make your life easier. This one allows you to set a label to a textbox, which redners before the textbox. Great use of compositing.

Except for one little detail...

Before I explain the problem let me just say... details like this are really frustrating. ASP.NET is very powerful, but with that power comes some responsibility. It is very easy to do things the "wrong" way. It's also very, very easy to do things the right way, but you have to know the difference. And knowing the difference means having a fairly deep understanding of how the framework, well, works. This and my previous post on ViewState are just two examples of these mundane yet crucial details.

And now on to the juicy stuff...

My problem with the example code is really a state management thing. When it creates the textbox and label, it "copies" the Title and Text properties, which are ViewState-based properties of the composite control, into them. That raises some red flags to me, because as soon as you are finished copying the values into the child controls, your public properties are now completely disconnected from them. If someone, somewhere, for some reason, changes your Title or Text property value after this code occurs, it's too late. They will be dumb-founded as to why your control refuses to listen to their instructions (it will still render to 'old' value).

THE SOLUTION

The solution is so elegant in my opinion, I'm not sure why this isn't official recommended practice for compositing. I call it "delegating the properties". It means, don't store the state yourself, use the child control itself to store the state. The real problem was we had the value of our properties stored in two locations -- our ViewState, and our child controls' ViewState. Using the child control itself to store the state means it will always be in just one place, a place where we the parent and the child control can agree on. Here's how:

private TextBox txtFoo;
public MyControl()
{
  this.EnsureChildControls();
}
public string Text
{
  get { return this.txtFoo.Text; }
  set { this.txtFoo.Text = value; }
}
protected override CreateChildControls()
{
  this.txtFoo = new TextBox()
  this.Controls.Add(txtFoo);
}

The Text property does nothing more than access the textbox's Text property. No longer do we need to 'copy' the value into the textbox -- its already there!

The important thing about this trick is to simply call EnsureChildControls() in your constructor. That's so the textbox will exist should someone try to set the Text property (which is very early on if they set it declaratively). Alternatively, you could call EnsureChildControls() within the get and set like so:

public string Text
{
  get
  {
   this.EnsureChildControls();
   return this.txtFoo.Text;
  }
  set
  {
   this.EnsureChildControls();
   this.txtFoo.Text = value;
  }
}

However, if you have several properties that do this, its much easier to just do it in the constructor. Less lines of code result.

Happy control building!!!

ViewState: Misunderstood monster or fluffy sleeping kitten?

The former!

Let me explain something to guys. ViewState is more complicated than you think it is. That's why you all abuse it so much :) Let me demonstrate:

public class MyControl : Control
{
  public string Text
  {
    get { return (string) this.ViewState["Text"]; }
    set { this.ViewState["Text"] = value; }
  }

  protected override void OnLoad(object sender, EventArgs args)
  {
     if(!Page.IsPostBack)
     {
        this.Text = "Hello World!";
     }
  }

  public override void Render(HtmlTextWriter writer)
  {
    writer.WriteLine(this.Text);
  }
}

What is WRONG with this picture?

Hmmmmmmm???

No.. that's not it.

You don't know?

I told you that's not it.

Give up?

That's what I thought. Sorry for my rudeness but it seems like no one on the planet knows why this is wrong. I have to deal with it every day as a lead programmer. But hey, I don't blame you. I've done this sort of thing myself. It looks perfectly harmless. You want the default value of "Text" to be "Hello World!". And why wouldn't you anyway, "Hello World!" has been serving us programmers since 1812. What better way to honor its 194th birthday than to make it the default value on your Text property?

No problem. Let it shine I say! But that's not what is wrong.

What's wrong is HOW you make it the default value. You know that HTTP is a connectionless protocol right? Well yeah, that's why Session state and ViewState were invented. To help you maintain data across those stateless requests. Brilliant, it really is! "Hello World" has never been so happy.

So let me ask you... What is this line of code for?

if(!Page.IsPostBack)

I know. It's so you don't do unnecessary work, right?

ViewState will be so kind as to save the value for you. Therefore, you'd be wasting CPU cycles if you had to set it on every request, right? Sure. What's that you say? Oh... You're also worried about overwriting the value in case someone has changed it? I see. Well that's pretty smart... I mean, afterall, ViewState is loaded before OnLoad. We wouldn't want to overwrite any precious changes to that value. Very good! Now wait just a second here...

You were on the right track, but you failed to understand the REAL problem here. The problem isn't that you would be wasting CPU cycles if you reassigned the value on every request...

Oh no. The problem is much greater than that.

The problem is that you are completely unaware of the purpose of ViewState. Did I say that already? Sorry, really I am...

Here... There's no better way than to show you.

public class MyControl : Control
{
  public string Text
  {
    get { return this.ViewState["Text"] == null ?
            "Hello World" :
            (string) this.ViewState["Text"]; }
    set { this.ViewState["Text"] = value; }
  }

  public override void Render(HtmlTextWriter writer)
  {
    writer.WriteLine(this.Text);
  }
}

What have we got here. Hmm... No OnLoad anymore.
"Hello World" has moved up into the property. If the value in ViewState is NULL we return it. That means, dun dun dun! We have a default value! And we didn't have to set it, its just THERE.

Ok... so big deal. Who cares?

I'll tell you who cares... and its not a fluffy sleeping kitten.

In the 2nd control, the value "Hello World!" is not persisted into the hidden form field "_VIEWSTATE". In the first control, it is.

Its a very... subtle... difference. But its VERY IMPORTANT!!!

I have seen controls written the "bad" way that contained dozens of properties. The result is, when someone puts this nasty control on a page, even BEFORE THE FIRST POSTBACK, their viewstate is HUGELY BLOATED with crap. If you do it the 2nd way, its just as big as if you had ZERO properties. If some unlucky Joe Programmer uses your control within a DataGrid or repeater, they'll be taking a hit once for every data item they bind.

VIEWSTATE IS NOT TO BE TAKEN LIGHTLY :)

Here's the rule of thumb that you must live and die by: VIEWSTATE IS FOR TRACKING CHANGES TO THE FORM'S STATE. Thats CHANGES. It is NOT meant to store the default values of your properties. Why on earth should it store default values? THEY'RE DEFAULT VALUES for crying out loud. Don't do it! :)

I feel much better now. Thank you! :)