Posts Tagged ‘Quality’

 

{{{{{

If you feel strongly that curly braces are best lined up in a particular style, then I have to wonder if you view code more at a low level than at a high level.

At a higher level, code could be thought of as assemblies made up of classes with statics, fields, methods, properties, and events. Curly braces are not very relevant at that level.

Quality code design happens at higher levels, not at curly braces level. Coding and design are usually inseparable at all but the most disciplined companies. Maintaining that higher-level focus while coding will yield better design.

Inconsistent curly braces or other syntactic styling, however, will distract other developers from the higher-level aspects of code, causing them to see the syntax before they see the higher-level constructs such as method, property, and class design.

Most good, experienced developers can get used to any team’s style rules in a week or two, no matter how illogical the rules may be.

Be consistent, and let those curly braces drift into the background.

}}}}}}

Most WPF developers know about the visual and logical trees of WPF. For example, a StackPanel with a Button may be made up of a StackPanel, Button, a ButtonChrome, a ContentPresenter, and a TextBlock. That’s the actual visual tree, but the logical tree is simply a StackPanel with a Button.

If you only specify your visuals in XAML, you may be overlooking another important control hierarchy. Consider this XAML:

<TabControl>
<TabItem Header=”Tab 1″>
<TextBlock Text=”This is on Tab 1″ />
</TabItem>
<TabItem Header=”Tab 2″>
<TextBlock Text=”This is on Tab 2″ />
</TabItem>
</TabControl>

The hierarchy shows that a TabControl has two TabItem controls, each with a TextBlock. This implies that the TabControl owns the TabItem controls, and each TabItem owns a TextBlock. Consider this code:

TabControl myTabControl = new TabControl();
TabItem myTabItem1 = new TabItem();
TabItem myTabItem2 = new TabItem();
TextBlock myTextBlock1 = new TextBlock() { Text = “This is on Tab 1” };
TextBlock myTextBlock2 = new TextBlock() { Text = “This is on Tab 2” };
myTabItem1.Content = myTextBlock1;
myTabItem2.Content = myTextBlock2;
myTabControl.Items.Add(myTabItem1);
myTabControl.Items.Add(myTabItem2);

Notice how flat the code looks when compared to the XAML. In the code, the same object that has a reference to the TabControl has a reference to each TabItem and TextBlock.

Imagine that you need to change the text on the TextBlock that is on Tab 2. With the XAML, you would need to walk the visual or logical trees to get a reference to the object. With the code, you already have a reference to all of the objects without walking any trees. You can simple write

myTextBlock2.Text = “New text for Tab 2”;

We really have three trees in WPF: the visual tree, the logical tree, and the ownership tree. With the code above, all of the controls have the same owner. If the owning object were a dialog, for example, the dialog may have references to every item on it. Therefore, it does not need to walk the visual or logical trees to get to its controls. The ownership tree is therefore flatter than the visual and logical trees.

I have seen developers spend unnecessary effort trying to make their business logic match the visual hierarchy of the controls. I would argue that this is counter-productive. Just because a dialog’s visual hierarchy has, say, ten levels does not mean that the business logic must have ten levels.

The business logic is mostly independent of the control-layout logic. It may not be entirely independent. For example, the dialog may not expose references to its controls to the outside world. Therefore, whoever owns the dialog does not own the controls on the dialog (unless they are intentionally exposed through a public API). Thus, the ownership hierarchy has a dialog at one level and all of the controls at a second level.

I thus recommend thinking in terms of three WPF trees: the visual tree, the logical tree, and the ownership tree.

Please feel free to comment with your own experiences.

Thanks,

Dale

Some developers pride themselves on writing flexible classes, making as few assumptions as possible about how other objects would use them. This sounds good, but is there such a thing as excessive flexibility?

“Excessive flexibility” implies that a class fails to do enough decision-making or encapsulation on its own, so you have to do it in the calling code. In an effort to provide a flexible class, a developer unintentionally provides a class that requires more work to use and allows less room for performance optimization.

I encounter business classes similar to the one below. This class opens a text file and provides a List<string> where each item in the list represents one line of text in the file.

public class SomeDataClass: IDisposable
{
string path;
StreamReader streamReader;

public SomeDataClass()
{
}

public SomeDataClass(string path)
{
Initialize(path);
}

public void Initialize(string path)
{
this.path = path;
}

public void Open()
{
if (streamReader != null)
streamReader.Close();

streamReader = new StreamReader(path);
}

public void Close()
{
streamReader.Close();
streamReader = null;
}

public void Dispose()
{
if (streamReader != null)
{
streamReader.Close();
streamReader = null;
}
}

public List<string> GetLineData()
{
var newList = new List<string>();

while (!streamReader.EndOfStream)
{
newList.Add(streamReader.ReadLine());
}

return newList;
}
}

The public API includes a constructor, Initialize, Open, Close, Dispose, and GetLineData. This class provides some flexibility features:

  1. You can create one using a convenient parameterless constructor.
  2. You can use the constructor overload that automatically initializes the object.
  3. You can open the file only when ready to read it.
  4. You can close and then reopen the file without creating a new class instance.
  5. You can call Dispose instead of Close if you prefer.

I would have some questions for a developer who wrote that code:

  1. Will you provide documentation on best practices when using this flexible class?
  2. Why does it need the complexity of two constructors instead of one?
  3. Does the object provide a useful service when constructed by not initialized?
  4. Does the object provide a useful service when initialized, but not opened?
  5. If Close and Dispose do the same thing, why have both?
  6. Could dispose-and-new-again replace the close-and-then-reopen functionality?

I simplified this class:

public class SomeDataClassSimplified
{
string path;

public SomeDataClassSimplified(string path)
{
this.path = path;
}

public List<string> GetLineData()
{
var newList = new List<string>();

var streamReader = new StreamReader(path);

while (!streamReader.EndOfStream)
{
newList.Add(streamReader.ReadLine());
}

streamReader.Close();

return newList;
}
}

This version provides the same functionality with only two methods. Does this class require documentation? Notice that instead of creating the StreamReader in the constructor, Initialize, or Open, it creates it only as needed. This optimization results from having more implementation flexibility inside the class whereas the first version provided more flexibility outside the class.

While I contrived this over-simplified example, the same concepts apply to large business objects.

I worked with an architect who likes the term “appropriate.” In the case of business-object flexibility, I am looking for “appropriate flexibility” in the public interface, which in most cases means, “as simple as appropriate.”

I offer a few guidelines when writing business objects:

  1. Avoid overloading constructors. A constructor should require those parameters it needs in order to reach a usable state to avoid the need for Initialize types of public methods.
  2. Avoid providing developers convenience methods that add no real value. In the first example above, just as in the StreamReader class written by Microsoft, the class provides two ways to do the same thing: Close and Dispose. A developer trying to use your class will need to read documentation to figure out how to use your class.
  3. Reduce the public interface to your class in order to provide more “wiggle room” inside the class for optimization.
  4. Write self-documenting public interfaces. The first example does not make clear the difference between the constructor, Initialize, and Open. The second example self-documents.
  5. Carefully consider the number of object states you allow. The first example has at least four valid states: (1) constructed, but not initialized, (2) constructed and initialized, but not opened, (3) constructed and initialized and opened, and (4) constructed and initialized, but closed/disposed. The second example has two states: (1) constructed and (2) disposed.
  6. Avoid exposing invalid object states. In the first example, constructing an object and then calling GetLineData would fail.  Similarly, closing the object and then calling GetLineData would fail. In complex applications, the more you expose opportunities to use classes incorrectly, the more often it will happen.
  7. If you do not immediately need a business object, do not construct it. Wait until you need it to consume the memory. Generally, dispose it as soon as you no longer need it to free resources. Write code that allows business object references to remain null most of the time.

Please add comments to refine these ideas for appropriate business-object flexibility or to show your support for these ideas.

Good luck,

Dale

Technorati Tags: ,

For ideas on when to implement IDisposable, see another of my blog posts, Implement IDisposable More.

If you decide that your class should implement IDisposable, you have done your part to be sure that resources can be cleaned up immediately with a call you your Dispose method.

However, what if another object fails to call your Dispose method? Your Dispose method will never run.

In most cases, if a Dispose method does not run, I consider it a coding error. To catch it, I developed this simple enforcement trick:

public class SomeDisposableClass: IDisposable
{
bool isDisposed;
StreamReader myStreamReader;

public SomeDisposableClass()
{
myStreamReader = new StreamReader(“c:\foo.txt”);
}

public void Dispose()
{
isDisposed = true;

if (myStreamReader != null)
myStreamReader.Dispose();
}

#if DEBUG
~SomeDisposableClass()
{
if (!isDisposed)
Console.WriteLine(“An instance of SomeDisposableClass was not disposed before garbage collection.”);
}
#endif
}

A private field called isDisposed defaults to false, but is set to true if the Dispose method runs. If the Dispose method does not run, a finalizer catches the coding error by outputting a message to the console window.

Objects with finalizers pay a garbage collection performance penalty. To avoid that penalty, the above code only compiles in the finalizer in debug mode.

Typically, you can spot these error messages in the console window when garbage collection runs at application shut-down. However, they can show up at any time if your object is garbage collected sooner.

If you really want to enforce that your Dispose methods get called, throw an exception instead of outputting a message to the console window.

Be careful what you do in your finalizer. The finalizer will run on a garbage-collection thread, so avoid the temptation to reference any non-thread-safe fields. While it would be nice to pop up a message box to report not-disposed errors, during application shutdown, you may or may not catch a glimpse of it before shutdown completes.

Please add comments to refine the use of IDisposable or to show support for this enforcement trick.

Enjoy,

Dale

Technorati Tags: ,

The following code snippet demonstrates the only way that I know of that the C# language specification treats IDisposable differently than any other interface:

// StreamReader implements IDisposable.
using (var myStreamReader = new StreamReader(“c:\foo.txt”))
{
Console.WriteLine(myStreamReader.ReadLine());
}

StreamReader implements the IDisposable interface and can therefore be used with the using C# keyword. At the end of the using block, the myStreamReader object’s Dispose() method will run automatically.

You may speculate that an object implementing IDisposable somehow pays a penalty, perhaps by demotion to a slower garbage-collection tier, but according to Microsoft, IDisposable works like any other interface in garbage collection.

Although C# does not penalize objects implementing IDisposable, the usual recommendation to let the common language runtime (CLR) garbage-collect them without worrying about calling Dispose methods makes good sense.

I read somewhere that you should implement IDisposable only if your class references unmanaged resources. I think implementing IDisposable can improve quality in several ways:

  1. For performance reasons, you wish to free a resource sooner rather than later. For example, while a managed database connection or file handle will close when the garbage collector collects the object, you may wish to free the resources as soon as your application no longer needs them.
  2. You need to call the Dispose method on sub-objects that you own.
  3. Ease verifying correctness when working with complex object relationships. If, in a Dispose method, you clear values for all or most of your fields, then if some other object is mistakenly continuing to reference a field on the disposed object, you will likely catch the error quickly as a null reference or out-of-range error. I have found this to be especially helpful when forgetting to unregister from an event. When the event handler runs, it usually references a cleared out field and breaks, thus alerting me to my coding error.
  4. And of course, if you have unmanaged resources, you should implement IDisposable to close/free/release them.
  5. ?
  6. ?
  7. ?

Do you use IDisposable for reasons not mentioned above? Please add comments to describe those cases.

Please add comments to help refine the use of IDisposable or to show your support of this post.

Thanks,

Dale

Technorati Tags: ,