Confused by my own design – behaviour through inheritance – How not to achieve it

I wonder, do we all get to that point where our own code, written days or weeks or months or maybe even years back, makes us ask the question, “How the fuck did I do that?”

Everything here refers to my RomyView application.

Take for instance, my conversion progress dialog. This is part of my video converter class library.

At design time, it looks like this:

ConversionProgressDesignTime

But at run-time, it looks like this:

ConversionProgressRunTime

Ignoring the colours being different, which is part of my custom Themes implementation, the noticeable difference is that somehow my custom code that paints shadows for buttons and the panel ran, even in the forms designer, but not for the ComboBox. Why?

It turns out that along the way I created quite a complex forms inheritance hierarchy, and this dialog inherits from Romy.Controls.BaseDialog, which inherits from Romy.Controls.BaseForm. And my base form class implements a paint handler like so:

Base Form Paint Handler
  1. protected override void OnPaint(PaintEventArgs e)
  2. {
  3.     if (InheritedDrawShadows)
  4.     {
  5.         foreach (var c in Controls.ToArray<Control>().
  6.             Where(cc => cc.Visible && ((cc is Panel && cc.GetType() != typeof(ShadowLessPanel) &&
  7.                 cc.GetType() != typeof(BlackPanel)) || cc is Button || cc is PropertyGrid || cc is Thumbnail)))
  8.         {
  9.             var drawShAadow = true;
  10.  
  11.             if (c is Thumbnail)
  12.             {
  13.                 var thumb = c as Thumbnail;
  14.  
  15.                 if (thumb.IsCustomSize && thumb.BorderStyle == BorderStyle.None)
  16.                     drawShAadow = false;
  17.             }
  18.  
  19.             if (drawShAadow)
  20.                 e.Graphics.DrawShadow(c);
  21.         }
  22.     }
  23.     base.OnPaint(e);
  24. }

 

What that says is, for all derived forms that don’t set InheritedDrawShadows false, for all buttons, panels, thumbnails and property grids (excluding certain derived custom types that I especially created not to have shadows), and unless the control is an instance of my Thumbnail control using a specific optimization that allows a Thumbnail just to draw an Image and nothing else, draw the damn shadow. This code is the reason that certain shadows are drawn even in the designer.

And now I remember, I didn’t add ComboBoxes to that code because I might not always want them to have shadows. In fact, usually they shouldn’t have. But I added an OnPaint handler to the progress dialog for that one.

OK. Now it makes sense. It also explains why the dialog below, part of my image filters dialogs class library in the solution, looks the same at design time and run-time:

ResizeDialog

But that doesn’t explain what happened here… This dialog, also part of the same class library, doesn’t do any of my drawing at design time:

HslDialogDesignTime

But at run-time, it’s behaving as it should be:

HslDialogRunTime

Actually, this reveals a limitation of my base class paint handler. It can only paint shadows for certain types of controls that are dropped directly onto the form.

Oops. And how did I make it draw the shadows above? With this horrible hack:

Base Dialog Paint Handler
  1. #region Protected Methods
  2.  
  3. /// <summary>Button shadow affects for dialogs.</summary>
  4. /// <remarks>Defined here, rather than the base form, because I typically
  5. /// have buttons in SplitContainers on dialogs, not forms. And if I do have
  6. /// any on forms, I may not want this default shadow drawing.</remarks>
  7. protected override void OnCreateControl()
  8. {
  9.     /* The base form’s paint handler doesn’t handle nested controls, thus it won’t paint the
  10.      * shadows for buttons on the split container panels. Painting them requires handlers for
  11.      * any SplitterPanels that contain buttons. We add them here. */
  12.     foreach (var s in Controls.ToArray<Control>().Where(ss => ss is SplitContainer))
  13.     {
  14.         foreach (var p in s.Controls.ToArray<Control>().Where(pp => pp is SplitterPanel && pp == ((SplitContainer)s).Panel1))
  15.         {
  16.             p.Paint += (sender, args) =>
  17.             {
  18.                 PaintPanelShadows(p, args);
  19.             };
  20.         }
  21.  
  22.         foreach (var p in s.Controls.ToArray<Control>().Where(pp => pp is SplitterPanel && pp == ((SplitContainer)s).Panel2))
  23.         {
  24.             p.Paint += (sender, args) =>
  25.             {
  26.                 PaintButtonShadows(p, args);
  27.             };
  28.         }
  29.     }
  30.  
  31.     foreach (var p in Controls.ToArray<Control>().Where(pp => pp is Panel))
  32.     {
  33.         p.Paint += (sender, args) =>
  34.         {
  35.             PaintButtonShadows(p, args);
  36.         };
  37.     }
  38.  
  39.     base.OnCreateControl();
  40. }
  41.  
  42. #endregion Protected Methods
  43.  
  44. #region Private Methods
  45.  
  46. private static void PaintButtonShadows(Control p, PaintEventArgs args)
  47. {
  48.     foreach (var b in p.Controls.ToArray<Control>().Where(bb => bb is Button && bb.Visible))
  49.     {
  50.         args.Graphics.DrawShadow(b);
  51.     }
  52. }
  53.  
  54. private static void PaintPanelShadows(Control p, PaintEventArgs args)
  55. {
  56.     foreach (var b in p.Controls.ToArray<Control>().Where(bb => bb is Panel && bb.Visible))
  57.     {
  58.         args.Graphics.DrawShadow(b);
  59.     }
  60. }
  61.  
  62. #endregion Private Methods

 

Conclusion

While this code works, it is rather flimsy. Controls in different containers will not inherit the intended behaviour at all.

So what did I do wrong?

While the approach was heading in the right direction, my implementation is, well, backwards. Instead of trying to paint every type I could think of from the forms, and going as far as deriving specific types that should not inherit the intended behaviour, I should have created types specifically to inherit the intended behaviour. That is, create derived Buttons and so on that should always paint shadows, no matter what their containers might be. It would have involved less code, and been easier to read as well.

I hope that this might point out to somebody else who is heading down the wrong road (or heading up the right road instead of down) that they need to rethink their design. These things are easy to get wrong while you are coding in that zone deep in the code…

Note: Obviously Graphics.DrawShadow is an extension method and is irrelevant to the point of this article.

Advertisements

About Jerome

I am a senior C# developer in Johannesburg, South Africa. I am also a recovering addict, who spent nearly eight years using methamphetamine. I write on my recovery blog about my lessons learned and sometimes give advice to others who have made similar mistakes, often from my viewpoint as an atheist, and I also write some C# programming articles on my programming blog.
This entry was posted in Programming and tagged , , . Bookmark the permalink.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s