July 2009 - Posts

According to the wikipedia definition, best practice is

…a technique, method, process, activity, incentive or reward that is believed to be more effective at delivering a particular outcome than any other technique, method, process, etc.

The word “believed”, in my opinion, plays an important role. What one believes in, is not necessarily shared by another person. And not only with faith, but with technologies as well. What one technology / platform / methodology will describe as good practice, another one can claim if not the opposite, then at least disagree on the stated.

The motivation for this though was withdrawn from the project I am working on quickly, which is entirely written in Java. Despite my opinions on Java as a language, there’s something fundamental embedded in  Java, that does not exist in C# – one class per file. It bothered me, so I decided to spike the history of the feature.

Now you can debate for hours if this right or wrong. I think this is a false best practice. From Object Oriented perspective, there’s nothing wrong with more than one class (or “type”) within a single file. Let me demonstrate a few examples.

Example 1 – Design by Contract

The system is coded with a Container (Dependency Injection), and Contracts are what we care about, not implementers. In some cases it makes sense to see packaging done the following way:

   1: public interface IContract
   2: {
   3:   //...
   4: }
   5:  
   6: internal class DefaultContractImplementation : IContract
   7: {
   8:   //...
   9: }

I intentionally highlighted “packaging”, because in my opinion it’s all about packaging and some sort of convenience.

Example 2 – BDD Specifications

If you are familiar with Behaviour-Driven Development, you know that there can be multiple specifications for a single SUT (system under test). The most common and efficient way to achieve it, is to break it into separate classes, extending some sort of base spec for a particular SUT.

   1: public abstract class DomainService_Specs : ContextSpecification<IDomainService>
   2: {
   3:     protected IDomainService system_udner_test;
   4:     // ...
   5: }
   6:  
   7: public class When_DomainService_is_asked_to_to_deliver_a_message_and_exception_is_thrown : DomainService_Specs

 

   9:     [Observation]
  10:     public void Should_recover_and_log_exception()
  11:     {}
  12:  
  13:     [Observation]
  14:     public void Should_do_something_else()
  15:     {}
  16: }
  17:  
  18: public class When_DomainService_is_asked_to_to_find_a_message_by_sender : DomainService_Specs
  19: {
  20:     [Observation]
  21:     public void Should_do_this()
  22:     {}
  23:  
  24:     [Observation]
  25:     public void Should_do_that()
  26:     {}
  27: }

We could potentially break the specs into multiple files, but is it worth it?

The Reason

Before making a blind assumption that a best practice is always good thing to apply, always look for the reason(s) it came to existence. Googling why Java had this best practice was an interesting finding (The Java Language Specification):

When packages are stored in a file system (�7.2.1), the host system may choose to enforce the restriction that it is a compile-time error if a type is not found in a file under a name composed of the type name plus an extension (such as .java or .jav) if either of the following is true:

* The type is referred to by code in other compilation units of the package in which the type is declared.

* The type is declared public (and therefore is potentially accessible from code in other packages).

This restriction implies that there must be at most one such type per compilation unit. This restriction makes it easy for a compiler for the Java programming language or an implementation of the Java virtual machine to find a named class within a package; for example, the source code for a public type wet.sprocket.Toad would be found in a file Toad.java in the directory wet/sprocket, and the corresponding object code would be found in the file Toad.class in the same directory.

It is obvious from the extract that the main reason behind the “best practice” was a technical optimization, and not OO principle implementation (like someone might raise up, Separation of Concerns, for example).

Conclusion

Java / C# (.NET) is a bit of a holly war. If you drop who borrowed what and when, you find that there’s an attempt to make a better development environment, and each attempt is packaged with the best practices considered to be “the best” at a time of product development. When product is outdated, or lagging behind, some will try to justify false best practices as best practices of even today. Be practical, search for the reasons, and make your decisions. Make sure you get the best out of the best practice, and not limited by it.

 

PS: Oh, and you can always convert to C# – proven to be more fun :)

Criteria involving multiple ORs and ANDs can quickly become ugly. David showed how some of our code became more readable by using a feature to join multiple ICriterion-s instead of using Restrictions class (as well as how to quickly leverage expressions to get away from using property names, and allow better refactoring by replacing strings with compile-able code).

James Gregory has provided earlier a very simple explanation on the subject. Absolutely love when things become simpler!

I ran into a problem once in a QA environment, and it was unpleasant to handle. Amr ElGarhy has showed a way “baked into” .NET how to overcome issues similar to the one I had – write emails straight into file system. Solutions is elegant and done through configuration file.

Convention over is defined in Wikipedia as follow:

Convention over Configuration (aka Coding by convention) is a software design paradigm which seeks to decrease the number of decisions that developers need to make, gaining simplicity, but not necessarily losing flexibility.

Our current project, that is no longer a skinny cow, has a state machine of message statuses, and a factory that can produce the state object from a key. Today I ran into a bug, when added two new message states, but forgot to update the factory, that happened to reside in a completely different assembly from the one that contains state classes. After a quick trace the bug was found. But how painful it is:

  1. As a developer I had to keep in mind to link between new message state classes and message state factory. This is getting easily out of control when you either new to the project or have not worked on it for a while. New developers have to be familiar with the limitation or find it out the difficult way.
  2. Each time a message state is added or removed to the state machine, factory has to be updated.
  3. Message state factory specification (tests) has to be updated as the production code, and again that’s a manual process that can lead to forgetfulness.

The conclusion was “we have to have A Pit of Success”. Convention over Configuration (or simply CoC) is what we need. So I spiked this quickly (no specs, sorry for that) and this is what is looks like.

Defining a “State”

public abstract class BaseState
{
  public abstract string Name { get; }

  public virtual BaseState MoveToNextState()
  {
    throw new NotImplementedException(string.Format("State '{0}' is not implemented.", Name));
  }
}

Base abstract BaseState class defines the default behavior and attribute for state name. A few state (three for simplicity) with only ability to move forward would look like the following code snippets.

image

public classInitialState : BaseState
{
  public const stringStateName = "Initial";

  public override stringName
  {
    get{ returnStateName; }
  }

  public overrideBaseState MoveToNextState()
  {
    return newIntermediateState();
  }
}

public class IntermediateState : BaseState
{
  public const string StateName = "Intermediate";

  public override string Name
  {
    get { return StateName; }
  }

  public override BaseState MoveToNextState()
  {
    return new FinalState();
  }
}
public class FinalState : BaseState
{
  public const string StateName = "Final";

  public override string Name
  {
    get { return StateName; }
  }
}

Fairly simple and straightforward. State factory, normally, would be a collection of all states, with a getter behavior that would allow from a key to get the state object. For simplicity, I am omitting the case when requested key has no equivalent state object.

public class StateFactory
{
  private static readonly Dictionary<string, BaseState> dictionary = new Dictionary<string, BaseState>
                                                                       {
                                                                          {InitialState.StateName, new InitialState()},
                                                                          {IntermediateState.StateName, new IntermediateState()},
                                                                          {FinalState.StateName, new FinalState()},
                                                                       };

  public BaseState GetStateFromName(string stateName)
  {
    return dictionary[stateName];
  }
}

This is the spot where the disconnection has happened. Each time a state is added or removed, dictionary has to be updates as well. So end this up, we can leverage CoC to reflectively discover statuses, and prevent future manual updates and forgetfulness (thanks David for a nice way of putting it in words).

Reflective Discoverer

The contract for reflective discoverer would accept 3 Funcs

  1. Function to return an array of assemblies to scan, determined at the time of invocation.
  2. Function to generate a key value, based on the item type.
  3. Function to verify is a type is matching the required criteria (could be predicate).
public interface IReflectiveDiscoverer
{
  Dictionary<key_type, item_type> Discover<key_type, item_type>(Func<Assembly[]> assembliesToScan,
                                                                Func<item_type, key_type> generateKeyFrom,
                                                                Func<Type, bool> verifyTypeCanBeUsed);
}

Implementation of the contract:

public class ReflectiveDiscoverer : IReflectiveDiscoverer
{
  public Dictionary<key_type, item_type> Discover<key_type, item_type>(Func<Assembly[]> assemblies_to_scan,
                                                                       Func<item_type, key_type> generate_key_from,
                                                                       Func<Type, bool> verify_type_can_be_used)
  {
    var dictionary = new Dictionary<key_type, item_type>();

    foreach (var assembly in assemblies_to_scan())
    {
      var all_types_in_assembly = assembly.GetTypes().Where(verify_type_can_be_used);

      foreach (var candidate_type in all_types_in_assembly)
      {
        var instance = (item_type)Activator.CreateInstance(candidate_type);
        dictionary.Add(generate_key_from(instance), instance);
      }
    }

    return dictionary;
  }
}

Now it’s time to update StateFactory to use ReflectiveDiscoverer.

public class StateFactory
{
  public BaseState GetStateFromName(string stateName)
  {
    var states = new ReflectiveDiscoverer().Discover<string, BaseState>(() => AppDomain.CurrentDomain.GetAssemblies(),
                                                                        state => state.Name,
                                                                        type => type.IsSubclassOf(typeof (BaseState)))
    return states[stateName];
  }
}

 

A few implementation details missing in this version:

  1. Factory is used frequently, and probably should keep states cached.
  2. We skipped specifications, and therefore have un-testable code.

Fixing those issues is not difficult. For testability we need to inject IReflectiveDiscoverer. For performance issue – keep the dictionary cached.

Final Version

private static Dictionary<string, BaseState> states;

public StateFactory() : this(new ReflectiveDiscoverer()) {}

public StateFactory(IReflectiveDiscoverer reflectiveDiscoverer)
{
  if (states != null) return;

  states = reflectiveDiscoverer.Discover<string, BaseState>(() => AppDomain.CurrentDomain.GetAssemblies(),
                                                            state => state.Name,
                                                            type => type.IsSubclassOf(typeof (BaseState)));
}

public BaseState GetStateFromName(string stateName)
{
  return states[stateName];
}

Parameterless constructor is replacing container functionality. “states” dictionary is a static member field that caches states with a guard clause in constructor.

Now, the code is cleaner and automated. No need to add states when those are added or removed. This will ensure that developers don’t make mistake of forgetting to update factory.

Finally, if we developers embrace the concept “People don’t fail, processes do”, our code will be better.

“Two better than one”.

“Once you try it, you never go back”.

Productivity can be achieved in multiple ways. One way is to provide developers with multiple monitors to minimize context switching and allow more real estate for concurrent usage. NEC Productivity Study lists a few factors users like about dual monitors:

  • Faster
  • More effective
  • Easier to use
  • More viewable space

Out company provides employees with laptops and an additional screen by default. So theoretically you end up with two monitors. The problem, is that laptop monitor is 15” and it’s resolution does not match (usually) larger monitors with higher resolution. Today, thanks to Victor, I have installed eVGA’s UV+ to enable 3rd monitor. Now I can really work. Each monitor has it’s dedication:

  1. Laptop – web for searches / build server status
  2. Monitor (middle) – project in Visual Studio I am currently on
  3. Monitor (left) – command prompt for local builds and/or tool(s) I need for work

This arrangement is a perfect win for myself. Considering that a human Field of View is no more than 140 degrees, 3 monitors are still in that range.

Now getting to the bottom of the business – cost. Managers are always worried about not wasting resources. In this case the investment is so minimal to get a serious ROI. A 17”-19” wide screen monitors are dirt cheap these days, below $200 mark. What’s that with in comparison to a developer that has less chances of making a mistake just because of a task (windows) switching.

Either way, I am definitely not going back. Build server will have to forgive me for taking the monitor away ;)

3.monitors

Update

It is possible to control position popups, dialog boxes, and other dynamic windows with various free applications. One application like that, TaskSwitchXP, can force task switch dialog window to be always showed on the active screen. There are no limits, the question is how limited your imagination ;)

image

image

Write your test code as you would write your production code. In my opinion, a few of the most critical things are:

  • Understanding (Readability and clarity of what the code is doing)
  • Easy of change (be able to reflect changes)
  • Quick safety net (understand what goes wrong and why)

In one of my previous posts, I have blogged about test naming conventions, that is popular among BDD adopters, and starts to pick up among classic-TDD followers as well. Seven months later, I can see where all the bullet items are missing in classical TDD, and present in BDD.

True story example

In BDD we have Specifications and Observations breakdown. The classical TDD is missing this aspect. So a specification like the follows,

   1: public class When_DomainService_is_asked_to_to_deliver_a_message_and_exception_is_thrown
   2: {
   3:     [Observation]
   4:     public void Should_recover_and_log_exception()
   5:     {}
   6: }

looks as next in classic TDD.

   1: ShouldRecoverAndLogExceptionWhenDomainServiceThrowsFaultExceptionAndDeliveryQueueServiceThrowsFaultExceptionWhenAttemptingToDeliverMessage

Readability / clarity is definitely compromised. A single word of 138 characters looks a bit complex.

Ease of change is not quiet simple in classic TDD case – you first have to de-crypt what the test actually does before updating test/production code.

Safety net – in this case we are on a thin ice – developers who have a problem to “de-crypt” the test code to figure out what went wrong, will eventually end-up either disabling tests for new code, or worse, removing old tests as well. And that happened in the past.

The moral of the post – BDD is all about the behaviour. Let the old habits of classic TDD-ism go away, accept that testing code is a first class citizen as the production code.

More Posts