When Over-Engineering Hits You Hard
Tags: dotnet, bestpractices, csharp, rant
Published: 2020-07-15 08:34:10

---
title: When Over-Engineering Hits You Hard
published: true
description: A few weeks ago, I wanted to use a really simple helper function from one of our companies internal NuGet packages. I just wanted to get a string representation of an enum-member that is defined in an attribute. But it wasn't as easy as it sounds.
tags: dotnet, bestpractices, csharp, rant
cover_image: https://dev-to-uploads.s3.amazonaws.com/i/gxcye5yt1lwz9vlf2w4m.jpg
---

I planned to write this post for a couple of weeks now. But recently I saw this discussion post from Ben:

{% link /ben/how-do-you-identify-over-engineering-1oad %}

This reminded me, that I should get back to this post and finish it.

## It sounded so simple

We use custom NuGet packages and an internal NuGet-server at work to make reusable code easily available. One of those NuGet packages is a "utilities" package, that contains functions and services that come in handy quite often.

A few weeks ago, I wanted to use the `EnumUtils`. As you can probably guess by now, it provides functions to work with `Enums`. For example, reading a string representation for an enum-member that is defined in a `Description` attribute. But let's look at an example. Let's assume we have an enum called `TicketStatus`.

```csharp
public enum TicketStatus
{
    [Description("open")]
    Open,
    [Description("in progress")]
    InProgress,
    [Description("testing")]
    Testing,
    [Description("approval")]
    Approval,
    [Description("done")]
    Done
}
```

And somewhere in my code, I want to get the description for a specific value of the enum.

```csharp
TicketStatus status = GetCurrentTicketStatus();
// now what?
```

So, now what? I knew that class was called `EnumUtils`. But `EnumUtils.GetDescription(status)` did not work. There is no `EnumUtils` class. But my IDE told me, that there is something called `IEnumUtils`. OK... I get it. We use dependency injection a lot. Let's get an `IEnumUtils` thing injected into the constructor.

```csharp
public class SomeServiceOrController
{
    private readonly IEnumUtils _enumUtils;

    public SomeServiceOrController(IEnumUtils enumUtils)
    {
        _enumUtils = enumUtils;
    }

    public string DoSomething()
    {
        TicketStatus status = GetCurrentTicketStatus();

        return $"Some message including '{_enumUtils.GetDescription(status)}'";
    }

    /* more methods here ... */
}
```

That was easy. And let me tell you something. It worked.

## Then, what's your problem? 🧐

Good question. 😄

My problems with this came up when I started writing the tests for my `DoSomething()` method. Here is what I did:

```csharp
[Fact]
public void DoSomething_IsWorkingFine()
{
    /* Doing stuff to create the test scenario... */
    
    var sut = new SomeServiceOrController(/* what to do here? */);
    
    var result = sut.DoSomething();
    
    result.Should().Be("Some message including 'in progress'")
}
```

What do I pass to the constructor of `SomeServiceOrController`? I already found out earlier that I cannot do `new EnumUtils()` because "EnumUtils" is not found. Sure, I could write a small hand-written mock or just use a mocking framework. This would totally work. But to be honest... **I don't want to do that**.

I mean... it's just a simple utils function. Why would I mock that? It just reads an attribute from an enum member. I see no need to mock that. That's why I checked the code of the utils package. And I found the following:

```csharp
internal class EnumUtils : IEnumUtils
{
    public string GetDescription(Enum value)
    {
        /* the implementation is not important here */
    }
}
```

Do you see it? It's `internal`. That's why I cannot use `new EnumUtils()` in my project.
And I thought "But what if I don't have dependency injection available? There might be smaller projects, where we do not have DI in place. How would I use it there?". So I looked a little deeper in the code...

```csharp
public interface IEnumUtilFactory
{
    IEnumUtils Create();
}

public class EnumUtilFactory : IEnumUtilFactory
{
    public IEnumUtils Create()
    {
        return new EnumUtils();
    }
}
```

First, the good part: Now I can use the factory in my test.

```csharp
var sut = new SomeServiceOrController(new EnumUtilFactory().Create());
```

Now I don't have to mock that stuff. Great. **But...**
An additional interface (IEnumUtilsFactory) and an additional class (EnumUtilsFactory) just to wrap the `new EnumUtils()`? Really? Why does this thing need a factory? I don't get that. 

## Time to calm down

I felt the urge to change the utils package immediately. This implementation is just wrong. I will fix it and the world will be a better place.

![calm down gif](https://media.giphy.com/media/xUA7b7pLM4w1edn0yI/giphy.gif)

After taking a short break and thinking about it a little more, my initial shock was gone. But another shock came. I realized that I did something I can't stand when others do it. I mean the "This is awful code. Why did anyone create it that way? I need to rewrite it to make it good." attitude.

You should always keep in mind, that the developer who created this code most likely had good reasons to do it that way. Maybe there were specific requirements. Maybe there were other constraints. Or they were just learning these concepts and thought it would be a good use-case for it. I mean, we have all been there, right?

## Final thoughts

Over-engineering will always happen. Over-engineering is real. Sometimes it is just a few classes that could be reduced to a simple function. Sometimes it is worse and the entire system architecture is excessively over-engineered. But again: There will be over-engineering and we have to deal with it from time to time. 

What about my scenario? I will talk to my colleagues about how we will deal with it. Maybe making the `EnumUtils` public would be a start. Or maybe keep the code as it is for now and create an extension method that does the same and can be used in new projects.

I think my takeaway is: Don't focus on the "bad code" and how you would have done it better. Focus on how to solve the problem at hand.