On good enough code

So what is good enough? 

Too many posts have passed without answering this obvious question.

Let me see the code. Let me ask you a few easy questions.

• Does it work?

Did I get you on the first hurdle? I hope not - but I've seen it too many times.

• Can I understand it? Even without thinking?

It is a code review, not deciphering contest.

• Is the structure clean and consistent?

Use similar name for similar things. Use different name for different things.

• Is it consistent with the rest of the project?

How does your change fit into the rest of the project?

• Are infrastructure parts separate? Reusable?

Hardly ever you will be able to reuse your business logic in a different project. But you can often reuse infrastructure. Or - if structured correctly - parts of it.

It is not necessary to always split everything into packages. But it should be simple to do later. 

• Do you have automated tests?

I don't like to set a target coverage. Test as much as is reasonably possible. Aim for 100% on any logic. Keep untested parts small and simple. I prefer functional tests using BDD over pure unit tests.

• And speaking about tests - do they really test enough?

Too often I have seen tests which did not test much. If the test method is called something like TestCalculate it is immediately suspect. 

Coverage analysis does not help you with this. It only tells you what is run, not what is verified. Test driven development helps a lot. 

• Can it be refactored easily?

Refactoring is a necessity to keep code in a good shape. Requirements will change. Bug will be found and fixed. Your code needs to be ready now. 

Did you answer yes to all of these? Congratulations, your code have a chance  to be good enough. No? Get back to a drawing board, this needs some improvement.

On infrastructure code 2

Quick recap from previous article: Infrastructure code is waste.

Today I want to talk about handling your waste properly.

Keep your waste in a trash can

Do not let waste pollute your code. Do not mix infrastructure with business logic. 

All starts with the single responsibility principle. When implementing business logic, questions arise. Can this still be considered 'single responsibility'? Or do I need to split the calculation? There are compromises to be made. And as long as the result is readable, it is sound approach.

This is not the case for infrastructure code. Even minor leak of infrastructure into your business logic makes huge damage. Your logic becomes 

• Difficult to test
• Difficult to maintain
• Fragile

Let's take an example here. We have a calculation, which needs to read current price from a WCF service.

interface IPricingEngine

{

    decimal GetPrice(string isin);

}

class Calculator

{

    public Calculator(IPricingEngine pricingEngine){/*....*/}

    public ResultType Calculate()

   {

       /* .... */

        var price = this.PricingEngine.GetPrice(isn);

       /* .... */

   }

}

This is correct. In real world, the pricing engine is not likely to be so nice. You might need to authenticate your calls. You might need to have a connection pool. You might need to try backup url or a different API version. You might want to log all calls.

This is all infrastructure. To implement any such requirement, you should never modify the Calculator class. Everything should be hidden within the IPricingEngine implementation.

Recycle

So you have IPricingEngine service contract and plain WCF client. Now you need to add all the infrastructure bits and pieces to make it work.

Wait... Logging... Connection pooling... Authentication.... Sounds quite independent on pricing engine, right? 

Right, you will need most of them again for your next service.

So make your waste recyclable. Make your infrastructure reusable. The most efficient way to build reusable infrastructure components is interception. See decorator and interception design patterns.

What's in it for me?

Your business logic is protected from all infrastructure peculiarities. Clean. Easy to change. Easy to test. Easy to fix.

Your infrastructure is a set of composable and reusable components.

There is no coupling between business logic and infrastructure. They can and will evolve independently.

On parameter validation

Many good developers think that the more you validate your inputs the better.

My favorite example of this is validation of constructor parameters.
Too often I can see a pattern like this:

public Client(string instanceUrl, string accessToken, string apiVersion, HttpClient httpClient)
{
    if (string.IsNullOrEmpty(instanceUrl)) 
    {
        throw new ArgumentNullException("instanceUrl");
    }
    
    if (string.IsNullOrEmpty(accessToken)) 
    {
        throw new ArgumentNullException("accessToken");
    }
    
    if (string.IsNullOrEmpty(apiVersion)) 
    {
        throw new ArgumentNullException("instanceUrl");
    }

    if (httpClient == null) 
    {
        throw new ArgumentNullException("httpClient");
    }

    _serviceHttpClient = new ServiceHttpClient(instanceUrl, apiVersion, accessToken, httpClient);
}

So we have one line of  useful code burried in 16 lines of noise. 

Not all parameter validation is like this, but constructor parameters are often a great example. Suppose you use dependency injection and a DI container like Unity. What is a chance it will ever send you null in your parameter?

Zero. 

So why are you validating it?

To look good. To look like a good, careful developer. There is no other purpose of this piece of code. Get rid of it. It is worth more dead than alive.

Or if you insist you really want to have it, are you willing to unit test it too? Are you really willing to add another 30+ lines of code to test these 16?

The main issue with this code is not that it is useless.  Neither it is how it violates single responsibility principle. Yes, being useless is enough to kick it out. Yes, violating single responsibility is a serious sin.

But it is actively risk-prone and thus harmful. Do you disagree? Fine. And did you actually spot the error in the code? There is classic copy-paste error there. Exactly the one you will have in your code sooner or later.

Don't get me wrong. Parameter validation is important. But understand what, how and why do you validate.

To be continued.

On infrastructure code

We all write some infrastructure code. All the debug messages. All the connection handling. All the reconnects of a failed connection. All the deployment scripts. All your little helper scripts. Yes your favourite ManagerHelperUtil class too.

Everything your client did not order. Everything your customer is not interested in.

We can call it infrastructure. But we can do better. Much better. The correct term is waste.

Waste? Are you serious?

Yes. All your precious infrastructure. All your cleverness. It is not what your client wants. It is a waste.

Waste?? Waste!! We need it! 

I know. We all produce waste. In our kitchens. In our lives. In restaurants. In offices. In factories. In software development.

It took us so much time! 

I guess so. And it still does. Code is a liability, not an asset.  Just let it go.

But it saves us so much more.

Great. Finally this is a good reason. Sometimes we just cannot avoid producing a waste. Or we could, but it is not worth it. Like 3D printers - no material wasted, but that does not make them efficient for mass production.

So we get to the roots. Yes, you can produce waste when you need. But still - think before you print. Think before you code. Keep the waste in the bin. Recycle.

On perfect code

Why to call a blog 'good enough code'? Why not perfect code? You don't want to have a perfect code?

Nope.

What? Are you serious?

Yes.
Let me tell you why...

We have all seen some good code. We have all seen piles of bad code. And some really really bad code. And we know bad code smells. But there is one code smell above all - perfect code.

Let me show you an example. I work in finance - sooner or later we will get there. But let me start with physics.

Let's say you are calculating amount of light Earth receives at any given moment.

public decimal CalculateLight(DateTime when) 
{
    /* ..... */
}

Sounds simple. You add up the sun, the moon, stars and you have the result.

What about solar eclipse? Lunar eclipse? 

Ooops... Ok, I can fix that.

Really? And what if solar eclipse happens during a full moon? Will it still work?

... take a pause now ... what is your answer?

1. Yes
2. No
3. Yes. It took me 3 weeks to implement correctly

Most developers will answer:

Well...  No....  Sorry give me that 3 weeks.. 

But actually the correct answer is

4. http://en.wikipedia.org/wiki/Solar_eclipse

Do you know already? This can never happen. Solar eclipse can only ever happen on a new moon. 

Busted - you have been caught in a a trap of perfect code.

There are edge cases which are not worth caring about. Perfect code solves them all. Especially these which will never happen in real world.

Ok, fine. But why it is the worst code smell?

How much time did you spend on it?
How much code did you add?
How much slower is it?

And again - for what benefit?

And that's why good enough is more useful than perfect. Which makes is better.

And that's why this is about good enough code.