Category Archives: Refactoring

Loose coupling: The Illusion

Posted in Best Practices, Encapsulation, Maintenance, Refactoring by

I must say I totally agree with the sentiment Jeremy has here. Loose coupling is an illusion to most of the projects have worked on. The project I current work on has the client UI in a distributed system knowing that we use NHibernate as our ORM. Unbelievable. Needless to saying Unit testing this solution is VERY hard! To me, this is the first place where loose couping rears its head. If you can not unit test a class due to concrete dependencies rearing their ugly head, then its either

  • Deal with the fact that you are not, in fact, loosely coupled or
  • Fix it.

As mention in Jeremy’s post having separate assemblies is not loose coupling. At best this forces a direction of flow of control, at worst it hides circular references or creates the illusion of loose coupling. Jeremy doesn’t break his solution down to quite the project granularity I do and nor do others (JP Boodhoo for example is know to have very few projects in a solution). The notion is that you are not trying to hide behind the perception of more assemblies == less coupling. You can also separate the single project into multiple assemblies in your build scripts if required. It then becomes a gentleman’s agreement amongst the team that coupling rules will be adhered too. Now this is much easier to police with a tool like NDepend.

Currently I am without NDepend so I still break up my solution to multiple projects, for a couple of reasons. I like to be able to visually see what is going on quickly and I like the default name spacing that is then applied (Sure this can be done with folders too). Probably the aspect I like most however is that i can see what reference what at any given time by checking the references (again, we don’t have NDepend on this project). By opening the UI project I can now see that Data Access references are made, or there are WCF references in a Data access project. Without NDepend this is my last resort to policing the silly things that go on in the current (and no doubt futures) project.

With NDepend I would certainly be moving toward smaller projects. *Thinking out loud* I think a common assembly for server side (with my default and abstract data access/repository stuff) a server side assembly and a client side assembly. It kinda makes sense. If none of that server side stuff will ever be accessed by an other application or assembly then why break it up? Hmmm…

Anyway, on the path to loose(r) coupling consider a couple of things:

  • Are you using a Test first approach. Although it is not necessary it tends to flush out dependencies early on
  • Are you using a means of Dependency injection? If you are new’ing up dependencies in line then you have just tightly coupled yourself to an implementations. Check out this for a start on DI including poor mans DI, which is still infinitely better than no DI, IMO
  • Code to interfaces not implementations. I always thought this was a pretty obvious statement, but apparently not. Your DEPENDENCIES should be interfaces. Anything you interact with in terms of Actions (ie methods or delegates/events) should ideally be via the interface. I very rarely see a point of having DTO’s implement an interface…
  • Stream line code that interacts with the unmockable (file system, DB, WCF/Windows services etc) ; it should be as thin as possible. Try to get your working code out of these classes. This is more of a testing issue, but will also lead to better design.
  • Get NDepend. It is a kick ass tool that i wish my boss would get for me 😦
  • Code reviews. Get out of trouble at the first sign, its very hard to “loosen up” an app once the tight coupling has set in.

Loose coupling is a design goal we should strive for but I believe it deserves a bit more than just lip service. Get the team on board an explain the benefits. The earlier this begins in the project timeline, obviously, the better

Gray hairs and coronaries …

Posted in AOP, Logging, NHibernate, Refactoring by

Refactoring fundamentals of a rather large stack ca be daunting… I gave it a go over the weekend, modifying hundreds of files and tens of thousands of lines of code and, to be honest, I thought I had done a pretty good job doing so.
Monday morning comes around and there where some Small build issues (csproj.user are not in the a build outside of VS) but easily fix.
The really problem was when we started to do complex interactions… the app died…

Oh sh1t…. I’m fired..

Well maybe not fired just roll back to Friday and I have wasted a weekend.
Long story short… don’t leave on Nhibernate logging… it kills your app.
Commenting out log4net returned everything to normal, now we just need to log the correct things (ie not every NHibernate interaction!)

The satisfaction of refactoring

Posted in Refactoring, Wcf by

I am in the process of heavily re factoring our service facade (ie WCF) layer. the first file i got hold over i have already removed 60% of the LOC and got a significant number of the methods to one line
eg:
public GeoLocationFindByIdResponse GeoLocationFindById(GeoLocationFindByIdRequest aTransportObject)
{
return FindEntityById<IGeoLocation>(aTransportObject.Payload,this.Domain.GeoLocationFindById);
}

that’s a big improvement from the 36 liner that it replaces and a whole lot easier to read.
One of our major issues was cut and paste code of logging and auditing/instrumentation. As we are not using AOP I have decided to go with passing delegates. It has actually sped up the method calls too as i have re factored out redundant methods calls and repeated logging. Now all of fault handling and the “aspect” like stuff can be handled in one place and the code is a whole lot easier to maintain.

Cool, i am happy with that!

Intention revealing interfaces

Posted in Best Practices, Design By Contract, Maintenance, Refactoring, Wcf by

In the argument of “Intention revealing interfaces” vs “Reusable DTO/message” i will 99% of the time lean heavily toward the “Intention revealing interfaces” side.

I have just encountered code where i can see a (somewhat) noble intention of trying to keep the number of messages available in an assembly down to a minimum. Considering the system(s) mainly passes entities across boundaries, message are not common currency.
Unfortunately i don’t think enough thought was put into the decision. Leaving the notion of passing entities around as another issue, the fact that this DTO is now being used as a request AND response object in MULTIPLE service functions means the intention of each DTO is very unclear.
There are multiple fields on the DTO that are not used by the service and there is a magical “ObjectPacket” property that is of type object (as in System.Object) that is actually quite important. The reason the property’s type was left as object is because of “reuse”, as the different functions do different things with that property.
OMG.
Many hours, nay days, of bug fixing could have been saved if this DTO was split in to just 4 classes, 2 response and 2 requests. These could have been generic enough for the remained of the functions and revealed, along with the service function names* the intention of the call.

Thinks of other when you names stuff (classes, components, service, methods and parameters) and think of whether re usability is actually a benefit before bundling on bonus properties to a transfer object. 😉

*yes this is a JBOS interaction

Time to Refactor

Posted in Refactoring by

Good sign you need to Refactor: Boxing and Unboxing making a prominent appearance… yuck.

With the use of generics and properly implemented interfaces this has been pretty much completely removed from one of our projects… my question is why was it there in the first place?