Tuesday, October 22, 2013

A lesson for IT Consultants from the Obamacare rollout

Much has been made in recent days of the shortcomings of the new Obamacare web site - Healthcare.gov; people are basically all asking the same question - why did they build it that way? The answer is because that's the way they were told to build it.

Picture the meeting. It shouldn't be hard. I've been in dozens of of them, and so have you. You're in a big room, and there are tons of people packed in there - some you know, most you do not. There are all the contractors who have been hired to do the programing. Some project management types over here. And scattered through the room are 1 - 3 representatives from every agency, department, and fiefdom that could have any possible interest, no matter how tangential, in the Affordable Care Act. All with their own agenda, and not one of them thinking about how to provide visitors to the website with the best possible online shopping experience.

To be fair, the programmers came in thinking about it a little, but their ideas are quickly brushed away in the avalanche of 'must have' and 'top priority' requirements as the consultants return to their Pavlovian response - "we'll do whatever you want". Why would they so quickly abandon their principles? Because they have been so abused by the bureaucracy they have forgotten that they owe the client, not only their typing skills, but also their best judgement.

Think about it. Every single thing on that web site is the specific request of somebody in that meeting. Having to sign up before viewing health coverage plans? Somebody demanded it. The 30 minute time limit for email to expire? A government policy. And so it goes.

So, fellow IT consultants, the next time you are tempted to take the easy way out and just give the client whatever they ask for, remember the Obamacare rollout fiasco. Nobody's blaming the bureaucrats who demanded these 'features'. All anybody will remember is that it's broken, and you built it.

Thursday, June 13, 2013

Exception Handling - Pet Peeves

I've seen so many ways that exceptions are misused in .NET code when it really should be so easy. I will lay out some recommendations that I have come up with through dealing with poorly written code and from reading everything I can find about exception handling in .NET. I readily admit that these recommendations are opinionated, but I feel like I have earned the right to my opinions from beating my head against the wall as I keep running into these issues.

The first rule of exception handling in .NET is DON'T! Unless you can fix the problem (hint: usually, you can't), just let the exception bubble up to the top layer of the app. Do not log the exception at this level. It gives you a false sense of having 'handled' it.

The second rule of exception handling is use a global exception handler (like ELMAH for ASP.NET, or your own exception handling class) to log the exception and notify the user. This should happen at the topmost level of the application.

I see this one all the time, and I can't understand it at all. Some people will do a try/catch and then re-throw the exception. Not only is this completely unnecessary; it is actively harmful. In a debugging scenario, execution will break where you catch it, not where the problem originally occurred making it that much more difficult to track down the problem.

Do not create your own exception types. I have never seen this done where it did not add complexity and obfuscate the real issue. Yes, it's theoretically possible to do it well, but resist the urge to try. Your fellow programmers will thank you.

NEVER swallow exceptions. NEVER, EVER, EVER. Unless you have thought very seriously about ALL the possible consequences. Even then, tell yourself, "I'm probably making a mistake."

I suppose a word about throwing exceptions is in order. 

First, apply the Samurai Principle - return successfully or not at all. If your method cannot do what its name says, throw an exception. Ironically, there are a couple of exceptions to this rule. When implementing either the TryParse or TryGet pattern or the NullObject pattern, you would return either a boolean or the NullObject.

Next, forget that soundbite: "Exceptions are for exceptional situations." I personally heard Jeff Richter say, "that's the stupidest thing I ever heard." He went on to explain that it's stupid if you take it in the sense of frequency. What it really means is that it is an exception to the rule. What rule? Well, whatever your method name is. Your method name is a contract. It says, "if you give me what I ask for (parameters), I will do whatever the method name is." If your method cannot complete its contract, throw an exception.

The first thing to do in your method is any kind of check or validation of the method's arguments. If something is wrong, fix it or throw an exception.

 Always throw the most specific exception type that accurately represents what happened. 


Now, if everybody will adopt my recommendations, it will make my life considerably easier. :)

The Samurai Principle Applied to Repositories

In general, I'm a big fan of applying the Samurai Principle (return successfully or don't return at all). Some things flow logically from this:

You never return null, and therefore never have to do null checks (Hooray!!!) You may decide to implement the Null Object pattern.

If your method cannot fulfill its contract (can't do what its name says), it should throw an exception. Forget that oft repeated mantra - exceptions are for exceptional situations. I personally heard Jeff Richter address that by saying, "that's the stupidest thing I ever heard."

Now, let's assume that we are using a repository type pattern in the data layer. This is basically a factory that returns an object or collection of objects based on querying a database. Standard procedure is to return a collection, populated by 1, many, or zero objects corresponding to the data found by a query. This is perfectly appropriate, and works well.

There is another case that is not so cut and dried - when you query the database by primary key expecting 1 and only 1 result. We have several options, but I'm not fully satisfied by any of them. Let's look at them.

1 - Treat it the same way as all the others, i.e. return a collection with 1 or zero objects.
This works, but just feels wrong.
2 - Follow the database model, i.e. return the object or null if not found.
I dislike this one.
3 - Use the Null Object pattern.
I like this one, but I have some reservations. Like, how do you keep the Null Object in sync with its counterpart?
4 - Apply the Samurai Principle, i.e. return the object or throw an exception if not found.
This appeals to the perfectionist in me.
5 - Find the object or create a new object, and return it.
I want to like this, but it seems to violate expectations.


Do you have any others or comments on these? Please post to comments.

Wednesday, June 12, 2013

Dependency Injection without IoC Containers

Dependency Injection does not mean using an IoC Container. In fact, Dependency Injection does not even mean using interfaces or base classes. Dependency Injection is all about the code entity (class or method) declaring its dependencies, and asking for them explicitly as arguments to be passed in to the constructor or method. The only alternatives to this are to do object creation inside your code or to use some kind of God object that has a reference to everything else. (This is basically what the service locator does. Sometimes, IoC Containers are used the same way.)

What's the benefit of using Dependency Injection as opposed to one of these alternatives?
1 - It makes for a better api. When you compose your methods and classes this way, you make the dependencies explicit. Your api is more informative. Without Dependency Injection, the api lies to you. It leads you to believe that these dependencies do not exist. To be clear, the dependencies still exist; they are just hidden. You have to look into the body of the method or class to ferret them out. 
2 - Single Responsibility Principle compliance. When your dependencies are not passed in as arguments, your class or method must assume an additional responsibility - that of creating its own collaborators.
3 - You create a seam that gives you a chance to write unit tests against this code.

One key to making this work well is to be explicit in what you ask for. It's the idea embedded in the Law of Demeter. Without going into all the details of the Law of Demeter, the basic idea here is you don't want to traverse the object graph looking for what you want. A silly example comes to mind.

public BadMethod(Car car)
{
FuelInjector injector = car.Engine.FuelInjector;
//code that deals with a fuel injector
}

public GoodMethod(FuelInjector injector)
{
//code that deals with a fuel injector
}

Another good practice is to define your parameters in terms of interfaces or a base class, and let the calling code or test instantiate the correct type. You have to be careful here. You need to be sure to use good object-oriented design, and not just create interfaces willy-nilly. The example I always remember is from John Somnez.

You begin writing your code like this:
class Kangaroo : IKangaroo
{
}
interface IKangaroo
{
}
class BoxingMatch()
{
public BoxingMatch(IKangaroo kangaroo)
{
}
}

I see this all the time. Especially when IoC Containers are being used. This is not the fault of an IoC Container, but people get into a habit of creating an interface for every class without thinking about it more deeply, just so it can participate in the IoC Container. Here's what it looks like if you carry it forward.

class MikeTyson : IMikeTyson
{
}
interface IMikeTyson
{
}
class GeorgeForeman : IGeorgeForeman
{
}
interface IGeorgeForeman
{
}

This way lies madness. The S.O.L.I.D. principles come to our rescue, in this case the Dependency Inversion Principle. You turn the normal way of looking at it on its head. It's the BoxingMatch class that 'owns' or 'publishes' the interface. The BoxingMatch class says, you need to implement the IBoxer interface if you want to work with me. You get something nice like this:

class BoxingMatch()
{
public BoxingMatch(IBoxer boxer)
{
}
}

interface IBoxer
{
}

class Kangaroo : IBoxer
{
}

class MikeTyson : IBoxer
{
}

So, where are we? Dependency Injection exists independently of IoC Containers. The reverse, however, is not true. Whether or not you use an IoC Container, you need to be careful and apply good object-oriented design skills.


Friday, June 7, 2013

Delayed answer to interview question

I recently interviewed with somebody who asked me, "how could you do tdd (test driven development) without mocks?", referring to a mocking framework of some kind. I mumbled something about our tdd process being immature (true), and some of our unit tests really being integration tests (also true), but the more I think about it, the more I think he had fallen into a trap I see all the time. For example, 

public class ClassA
{
private ClassB b;
public ClassA(ClassB b)
{
this.b = b;
}
public void Method1()
{
//logic that operates on data in this.b
}

public void Method2()
{
//some code here
b.SomeMethod();
}
}

Now, ClassA.Method1 is perfectly testable as is - no need for mocking frameworks, IoC Containers, service locators, etc. Just var b = new ClassB(), set up the data, and test away - like this:

[Test]
public void TestMethod1()
{
//arrange
var b = new ClassB();
b.SomeProperties = SomeData;

//act
var a = new ClassA(b);
a.Method1();

//assert
Assert appropriate state change.
}

[Test]
public void TestMethod2()
{
//Requires creating an IClassB interface and refactoring ClassA to take an //IClassB
var mockB = YourFavoriteMockingFramework.GetMock();
//set up mock object

var a = new ClassA(mockB);
a.Method2();

//assert
Assert that mockB.SomeMethod() was called.
}

I would argue that not only is TestMethod1() a better (more valuable) test than TestMethod2(), but ClassA.Method1() is better code than ClassA.Method2(). Why? Well, the whole idea of object-oriented programming is that the data and the methods that operate on that data live together in an object. It's entirely possible that the call to ClassB.SomeMethod() belongs somewhere else.

There will certainly be classes whose collaborators will be more involved than the example given, but to the extent you can keep logic in classes that depend on data and leaf objects, your system will be simpler, easier to test, and you may even be able to say, "Yes, we did tdd without a mocking framework".


*Notice that I didn't even use an interface for the dependency injection. I also think there's an over-reliance on IoC Containers, but that's another blog post.