Category Archives: Programming

Have Your Cake

This one’s for Ted, Joe, Vlad, Alan and anyone else who thinks functional programming is the shit.

This is neat stuff, because it means we’ll get most of the cool stuff from functional languages in a statically-typed, IDE friendly language. One of the most common complaints about Ruby that I’ve heard, both on our project and online, is that there is no good IDE support (meaning refactoring, and navigation through the code in this context). If this sort of stuff is coming to C#, you can also be damn sure that similar features will make it into Java.

When the Code Speaks

I did a little bit of refactoring the other day that I’m pretty happy with, so I thought I’d share.

We started out with a method in the SystemProfile who’s signature is specified below:


List createPriorityAllocationGroups(
ProfileGroup profileGroup,
List allAllocationChildren,
FacilityMonthlyMeasurements sourceForMeasurementPoints,
FacilityMonthlyMeasurements sourceForOwnerTypeClassification)

It is called within a loop for every ProfileGroup contained in the SystemProfile like so:


private List getProfileAllocationGroupsForProfileGroups(
List profileGroups,
List allAllocationChildren,
FacilityMonthlyMeasurements sourceForMeasurementPoints,
FacilityMonthlyMeasurements sourceForOwnerTypeClassification){
List profileAllocationGroups = new ArrayList();
for (Iterator iter = profileGroups.iterator(); iter.hasNext();) {
ProfileGroup profileGroup = (ProfileGroup) iter.next();
profileAllocationGroups.addAll(
createPriorityAllocationGroups(
profileGroup,
allAllocationChildren,
sourceForMeasurementPoints,
sourceForOwnerTypeClassification,
isCustomProfile));
}
sortProfileAllocationGroups(profileAllocationGroups);
return profileAllocationGroups;
}

Having to pass in four parameters to a method is a little smelly, but not too bad. The bad part happened within the method, which had grown quite complicated over a long period of time. The purpose of the method is to simply assemble a list of new PriorityAllocationGroup objects, so it should have looked something like this:


PriorityAllocationGroup allocationGroup = createNewPriorityAllocationGroup(group);
allocationGroup.addAllocationChildren(getPriorityAllocationChildrenForGroup(group));
allocationGroup.addMeasurementPoints(getMeasurementPointsForGroup(group));
return priorityAllocationGroup;

Instead, we had an implementation that was about 75 lines long, and so convoluted it took
me and my pair almost half a day to figure out how to implement a feature for our story.
Which sucks.

The very first refactoring I identified was to move the method into ProfileGroup. Many of the questions being asked within the method were specific to this object, so keeping it in SystemProfile no longer made sense. This involved moving a lot of other helper methods into ProfileGroup as well. Nothing actually got any simpler, but at least we were now just asking ProfileGroup to assemble it’s ProfileAllocationGroups instead of stuffing it all in SystemProfile.

So originally we had this:


class SystemProfile{
List getProfileAllocationGroupsForProfileGroups(
List profileGroups,
List allAllocationChildren,
FacilityMonthlyMeasurements sourceForMeasurementPoints,
FacilityMonthlyMeasurements sourceForOwnerTypeClassification);
List createPriorityAllocationGroups(
ProfileGroup profileGroup,
List allAllocationChildren,
FacilityMonthlyMeasurements sourceForMeasurementPoints,
FacilityMonthlyMeasurements sourceForOwnerTypeClassification);
}

Which became this:


class SystemProfile{
List getProfileAllocationGroupsForProfileGroups(
List profileGroups,
List allAllocationChildren,
FacilityMonthlyMeasurements sourceForMeasurementPoints,
FacilityMonthlyMeasurements sourceForOwnerTypeClassification);
}


class ProfileGroup{
List createPriorityAllocationGroups(
SystemProfile systemProfile,
List allAllocationChildren,
FacilityMonthlyMeasurements sourceForMeasurementPoints,
FacilityMonthlyMeasurements sourceForOwnerTypeClassification);
}

The next refactoring involved removing another smell (at least to me); passing in Lists as
parameters. Obviously there are sometimes reasons for this, but it’s usually better to be able to call a method like getAllAllocationChildren() at will instead of passing the list all over the place. As it turned out, the allAllocationChildren parameter originated from a method within SystemProfile. Since we were passing the SystemProfile in as a parameter, we could eliminate this parameter to make the method signature look like this:


class ProfileGroup{
List createPriorityAllocationGroups(
SystemProfile systemProfile,
FacilityMonthlyMeasurements sourceForMeasurementPoints,
FacilityMonthlyMeasurements sourceForOwnerTypeClassification);
}

One of the patterns I saw after this cleanup was that the 4 parameters originally passed in were in turn being passed in to other methods. Over and over again. Eventually the lightbulb goes on in my head as I realize that what we’re actually signifying by passing these parameters all over the place is that we’re operating inside a specific
context. So the next refactoring was to combine the sourceForMeasurementPoints, sourceForOwnerTypeClassification, and the SystemProfile into a
ProfileAllocationGroupContext object. Once this was done, it was a simple matter to change the signature of all the helper methods to take the context object as well.


class ProfileGroup{
List createPriorityAllocationGroups(
ProfileAllocationGroupContext context);
}

It’s now the job of the SystemProfile to create the ProfileAllocationGroupContext, which is then passed on to every ProfileGroup when we ask for its ProfileAllocationGroups. Whew!

This seemed ok, but there was still some stuff that didn’t quit fit. There were lots of calls like this in ProfileGroup.createPriorityAllocationGroups:


context.getMeasurementPointsToBeAllocated(this);

This was a fantastic example of how when you’re refactoring code it sometimes "speaks" to you. I had originally thought that putting this code in ProfileGroup was the right thing to do, but as I worked through all the above refactorings it became obvious that it wasn’t. Here’s the method that kicked everything off in SystemProfile as it stood at this point in the refactoring:


List getProfileAllocationGroupsForProfileGroups(
List profileGroups,
ProfileGroupAllocationContext context){
List profileAllocationGroups = new ArrayList();
for (Iterator iter = profileGroups.iterator(); iter.hasNext();) {
ProfileGroup profileGroup = (ProfileGroup) iter.next();
profileAllocationGroups.addAll(profileGroup.createPriorityAllocationGroups(context));
}
sortProfileAllocationGroups(profileAllocationGroups);
return profileAllocationGroups;
}

The code was telling me that instead of


profileGroup.createPriorityAllocationGroups(context)

we should actually be doing


context.createPriorityAllocationGroups(profileGroup)

This makes sense, if we say that for the given context, we want to assemble a list of
PriorityAllocationGroups. By moving the method to the context object we’re actually making it do double duty as an assembler or builder, but that can easily be factored out later. So in the end, my object model looked like this:


class SystemProfile{
List getProfileAllocationGroupsForProfileGroups(
ProfileAllocationGroupContext context);
//plus a bunch of state and other methods
}


class ProfileGroup{
//original state based stuff
}


class ProfileAllocationGroupContext{
List createPriorityAllocationGroups(ProfileGroup profileGroup);
}

Responsibilities of Objects

Here’s a quick refactoring tip.

Try to turn something like this…


public class Facility{
private boolean isGasBaseProduct(MeasurementPoint measurementPoint) {
return measurementPoint.getProduct().isGas()
|| measurementPoint.getProduct().isGasLiquid()
|| measurementPoint.getProduct().isGasByProduct();
}
}

…into this:


public class MeasurementPoint{
private boolean isGasBaseProduct() {
return getProduct().isGas()
|| getProduct().isGasLiquid()
|| getProduct().isGasByProduct();
}
}

Fun With Generics and Anonymous Methods

Here’s a little snippet of pure C# 2.0 sex that shows how to use anonymous methods and generics to create a facade into your domain layer. But first, a little background…

Layers are as follows, top down:

  • UI layer. Utilizes DTO’s assembled from the domain that directly map to a screen. All the screen does is display the values from the dto, nothing else. Testing the DTO’s gets you really close to the skin of the UI.
  • Facade. Organizes calls to the domain along functional lines. Wraps calls to the domain in a transaction, catches errors and warnings and sticks them somewhere accessible to the UI. Assembles domain objects into DTO’s for the UI. Updates the domain with info from the UI.
  • Domain. Business objects that perform the reall meat and hold all the logic pertaining to the customer’s problem.

In my case, the domain is mapped to a database using NHibernate. I’m using a Repository object to abstract that stuff away. Originally, my facade directly used the Repository in the following pattern:

public void AddBasePackageOptionsToCart(int unitId, int basePackageId, DateTime currentDate)

        {

            using (IRepository repository = DataManager.StartTransaction())

            {

                Unit unit = _FindValidUnitById(unitId);

                BasePackage basePackage = unit.Floorplan.GetBasePackage(basePackageId, currentDate);

                unit.Cart.SelectBasePackageAndAddOptions(basePackage);

                repository.VoteToCommit();

            }

        }

This is nice and clear, althought what do we do if an exception is thrown? Wrap each and every call in a try/catch? This is ugly, and creates repetition. We also have to remember to commit the transaction every time we are using a read/write transaction. I’ve spent much time debugging only to find I forgot to call VoteToCommit(). We can do better.

Enter the FacadeTransaction object. Using generics and anonymous methods, we can make sure our call to VoteToCommit is always called. We can also centralize our exception handling in one single place. Witness:

internal class FacadeTransaction<T>

    {

        public delegate T TransactionMethod();

 

        public TransactionMethod method;

 

        public FacadeTransaction(TransactionMethod o)

        {

            method = o;

        }

 

        public T Execute()

        {

            using (IRepository repo = DataManager.StartTransaction())

            {

                T returnValue = method();

                repo.VoteToCommit();

                return returnValue;

            }

        }

 

        public T ExecuteReadOnly()

        {

            using (DataManager.StartReadOnlyTransaction())

            {

                return method();

            }

        }

    }

And here’s how we utilize the FacadeTransaction:

public BasePackageListPageDto FindBasePackageListPageDto(int floorplanId, DateTime date)

        {

            return new FacadeTransaction<BasePackageListPageDto>(delegate

                {

                    Floorplan floorplan = Floorplan.FindById(floorplanId);

                    return new BasePackageListPageAssembler(floorplan, date).AssembleDto();

                }

                ).ExecuteReadOnly();

        }

 

        public BasePackageDto SaveBasePackage(BasePackageDto packageDto)

        {

            return new FacadeTransaction<BasePackageDto>(delegate

                {

                    BasePackageAssembler assembler = new BasePackageAssembler();

                    BasePackage package = assembler.Initialize(packageDto);

                    DataManager.GetCurrentRepository().Save(package);

                    return assembler.AssembleDto(package);

                }

                ).Execute();

        }

We use generics so that we can dynamically change our return type without casting. We use an anonymous method so that we can wrap each interaction with the domain in a single transaction without repeating code. We could also centralize our error handling if we wanted to. Sweet.

DTOs vs. MVP

I’ve been spewing a lot of mental diarrhea into the porcelain veneer of my blog over the last couple of days, but I’ve still got at least one more post to make before I revert back to my regular twice-per-month posting schedule. Here goes…

As I’ve stated in other posts, on my current project we generate screen specific DTOs from our facade layer that are then directly consumed by the presentation layer. Everything our GUI needs is within those DTOs (and hopefully nothing more). We write our tests (almost 12,000 of them now) to directly assert on the values contained in those DTOs, making our presentation layer as thin as possible. In essence, we’re attempting to test as close to the GUI as we can, which should surprise nobody. Since the GUI and all that it does is specified directly by the clients, testing our DTOs should go a long way towards making sure they are actually getting what they are paying for. We then run some watir-driven smoke tests before each checkin to make sure nobody accidentally forgot to change a property reference in the JSP.

Now there’s been a lot of hype, in the .NET community at least, regarding the Model-View-Presenter pattern. I actually like the pattern itself, but my main question lies in whether it should be used as a replacement for a pattern such as our “screen DTO,” or does it work with it?

Jean-Paul Boodhoo of Thoughtworks recently did a screencast on MVP, and as far as I can tell he’s using the MVP pattern in addition to a screen DTO approach. It seems like you’d have to first assemble the DTO below the Facade, and then essentially assemble the View directly from the DTO. It seems to me like the Presenter is actually just an assembler sitting outside the facade. So why have both? Am I missing something?

Writing Tests Top Down

One bit of info that I’ve seen floating around on the ‘Net revolves around the idea that if you’re doing test-driven development you should start from the bottom up, writing tests for small pieces of functionality and then moving up through the layers. This sounds good, and I’m sure it works for some people, but I’m coming to the conclusion that it’s not right for me.

I was re-reading some of the older posts on Ted’s blog, and came across an article he linked to titled Test Driving Code: Top-Down or Bottom-Up? which made me think a little bit. My executive summary of the article is as follows…

  • Top-down testing makes you focus on the specification of the business requirements of the code. The implementation becomes less of a distraction. Once your test is finished and passing, it becomes easy to refactor without tripping over your implementation.
  • Bottom-up testing locks down your implementation to a certain degree. If you need to change your model for some reason, you also need to change your tests.

To give some background, my current project writes almost no unit tests. We write functional tests on DTO’s that are passed into and returned from a Facade layer. Our goal is to have each DTO map exactly to the system it’s targeting, which is most often the Presentation layer. Therefore, we’re effectively testing the very thin, logic-free (we hope) portion of the app that’s closest to the user. We don’t care how the DTO gets it’s data, just that the data that is there is correct. The end benefit is that we can easily refactor the domain model without tripping on our tests. The downside is that when a test fails, it doesn’t tell us where it’s failing, just that the client’s specification of what they want to see isn’t being met. Which means we spend a lot more time in the debugger than we otherwise might. It’s a tradeoff.

From my understanding, we largely got to this point through first-hand experience, as detailed by Ted in some of his posts:

I don’t have enough experience with writing actual unit tests to have a valid opinion on this matter, although I do know that I tend to think in a top-down manner, which fits in well with the way we write our functional tests. Now is that way of thinking natural to me, or a side-effect of coming to a project that already worked that way? I suspect I won’t know for a while.

Working on a Large Agile Project

I commented on a post by Sam Gentile today, and his response posed a couple of questions that I figured were best answered on my blog. He had a couple of questions regarding how our team at CGI functions with so many developers (we have around 30, I think).

How are you managing the sitting together and keeping up the communications needed with that many people?

Currently, we have the entire team (developers, business analysts, testers, and one project manager) occupying 1/2 a floor of an office building. As the team gets bigger, they keep on literally knocking down walls for us.

We have 5 teams of approximately 7 or 8 developers sitting in ‘pods’ that contain 4 pairing stations (dual 19″ flatscreens, multiple keyboards and a pretty hefty development box) and two laptops. Each team is responsible for a certain area of the code, although team members swap out and there aren’t many issues with code ownership. Each pod is flanked by 2 desks where the BA’s from the client sit, and 2 others where QA sits. We do a standup in the morning for each pod separately, and then the team leads do another separate standup immediately afterwards.

Each sprint, the BA’s spend some time hashing out the functionality they require and turning them into stories for us to implement in the next sprint. They will often sit down with a developer to determine the details of the story. When a new sprint starts, we get a brief overview of the new stories, of which there are usually about 15. We then pair up, and get to work. At this point in the project, a pair will usually complete about 4 stories, or one per week. I’m guessing that our stories are quite a bit larger than what is standard for an agile project, however this project has been going on for over two years now, and we’ve kind of arrived at our sweet spot.

All that being said, we have excellent communication between the teams and the clients because we’re all in the same “room” together all day, every day. I’m not sure what was required at the beginning of the project to get all the clients (there are 4) to agree to having their BA’s offsite working directly with the project as I wasn’t around back then (Ted?). That must have been an interesting negotiation.

Have you considered writing an experience report for the Agile conference or an article?

I haven’t, personally, although I’m aware of the possibility that some interested individuals at the University of Calgary are intrigued by the project. You can check out Ted O’Grady’s blog for some more insight into the workings of the project, as he was around at the beginning and has been blogging about it on and off for a while now.

Specification Pattern With Predicates

I’ve been sick the last two days (I got a cold during the hottest part of the year so far, go figure) so I had some time to play around with C# 2.0′s generics implementation. What originally got me started was having an old project lying around that had out of date subversion information. Rather than going through each and every sub-directory and deleting .svn directories, I figured I’d write a little one-off app to do it for me.

I started out by attempting to implement the specification pattern, which led to various attempts at generic implementations that were never quite right. Fortunately, while searching for information on generics I stumbled across Joe Walnes post on The power of closures in C# 2.0 which details the use of predicate generic delegates.

For those not familiar with the specification pattern, it basically just allows you to filter a list of objects while keeping your condition separate from your loop. As an example, we often see something like this:

    1 public IList FindAllMaleCustomers()

    2         {

    3             IList matches = new ArrayList();

    4             foreach(Customer customer in Customers)

    5             {

    6                 if(customer.IsMale)

    7                 {

    8                     matches.Add(customer);

    9                 }

   10             }

   11             return matches;

   12         }

The specification pattern removes that customer.IsMale check out into a specification object, so we can now write something like this instead:

   42 public IList FindAllMaleCustomers()

   43         {

   44             return new CustomerFinder(Customers).FindAll(new MaleCustomerSpecification());

   45         }

Therefore, if the definition of a “Male” ever changes (bad example, I know) we can just change our MaleCustomerSpecification object.

Fortunately for me, .NET 2.0 includes a new type of delegate called the predicate generic delegate, which

Represents the method that defines a set of criteria and determines whether the specified object meets those criteria.

Seems like it’s exactly what we need. So rather than writing a specific specification object, we can just write a generic predicate like so:

    1     public class Spec

    2     {

    3         public static Predicate<DirectoryInfo> SubversionDirectory

    4         {

    5             get

    6             {

    7                 return delegate(DirectoryInfo dirInfo)

    8                           {

    9                               return dirInfo.Name == “.svn”;

   10                           };

   11             }

   12         }

   13     }

Which we then use like this:

    1 new DirectoryFinder(directories).FindAll(Spec.SubversionDirectory);

Our DirectoryFinder object is a little more complex than a standard FindAll on a method like System.Collections.Generic.List would be, because we recurse all the way through the sub-directories of a given directory:

    1 public class DirectoryFinder

    2     {

    3         private IList<DirectoryInfo> _directories;

    4

    5         public DirectoryFinder(IList<DirectoryInfo> directories)

    6         {

    7             _directories = directories;

    8         }

    9

   10         public DirectoryFinder(DirectoryInfo directory)

   11         {

   12             _directories = directory.GetDirectories();

   13         }

   14

   15         public IList<DirectoryInfo> FindAll(Predicate<DirectoryInfo> predicate)

   16         {

   17             return FindAll(predicate, _directories);

   18         }

   19

   20         private IList<DirectoryInfo> FindAll(Predicate<DirectoryInfo> predicate, IList<DirectoryInfo> directories)

   21         {

   22             List<DirectoryInfo> matches = new List<DirectoryInfo>();

   23             foreach (DirectoryInfo info in directories)

   24             {

   25                 if (predicate(info))

   26                 {

   27                     matches.Add(info);

   28                 }

   29                 else

   30                 {

   31                     matches.AddRange(FindAll(predicate, info.GetDirectories()));

   32                 }

   33             }

   34             return matches;

   35         }

   36     }

I’m almost certain there’s other ways of doing this, some are probably better as well. For another use of predicates, check out Jean-Paul Boodhoo’s post on validation in the domain layer, where he uses them to flesh out business rules.

Expand Your Brain

I just read The Nature of Lisp on defmacro.org, which attempts to explain why Lisp is so rad. I have to say, it’s by far the best article I’ve read in a while. I actually feel smarter, which is not a common feeling for me after reading anything related to Lisp.

As it turned out, the above article was a good lead-in to Sahil Malik’s series describing some of the good new features C# 3.0 will have. C# will definitely be getting some sweet features previously only really seen in dynamic languages (to my knowledge) in the future. Can’t wait.