On over-engineering; finding the right balance

I've never been bitten by an interface that is too simple. I've never had a bug in code that I didn't need to write.

Don't try to solve future problems. You'll have the problem that the problem you solved isn't the problem that needs solving, and then the problem that needs solving.

If anything, overly simplistic solutions sometimes make me feel like I'm repeating myself, but really since the code is so easy it always feels possible to refactor it. That's a good place to be, much better than afraid to change something because it's too generic to know if you'll break something.

Everyone has an opinion on an interface that is easy to understand, and not over-engineered (bikeshedding).

The key superpower is a culture of refactoring when needed.

I firmly believe you should design for the problem you've got now, and not worry too much about the future. You're usually wrong about it anyway! This is fine, as long as you do refactor when more requirements come along.

In order to do that you need a few things though: people who notice and care "oh we do the same in 5 places now, I should probably pull that out to a new module", a development culture that encourages and accepts that thinking, and a QA/delivery approach that doesn't say "oooh, that's a bit risky, do you have to refactor that?".

The right balance will look very different in different parts of a system. In general the lower levels of a system need to be more generic, and higher levels more specific. One often overlooked guiding principle is that when you’re building large systems it’s important to be able to “finish” some parts of the system and be “done” with them, otherwise your mental “context window” will just grow forever until not even the brightest among us can make any progress.

The thing missing from this analysis is how hard things will be to change later.

If it's an internal function called in just one or two other files maintained by you our your team, then no need to spend a lot of time thinking too hard about it -- just write it as simply as possible, and then rewrite it when you find it no longer suits your needs.

If it's going to go into an storage schema (database, json, whatever) which will require data migration if we need to refactor it -- or if it's an library that will be used by large amounts of code maintained by other teams or other organizations -- then it's worth spending a bit more time designing it. You're balancing the risk of wasting effort over-engineering it against the risk of the effort invovled in having to modify it later.

If it's going to be a public interface used by customers and supported indefinitely, then you'd better be very careful about getting it as close to perfect as you can, since now you're balancing the risk of wasting effort over-engineering it against the risk of having to support a terrible API for years.

I constantly try to improve how I write my code. The last couple of years I’ve been involved in a couple of projects that’s required rebuilding a few times over because the project managers kept changing directions and was always in a hurry.

For one project I eventually managed to convince them to let our team write a more general purpose library to conserve time in the future. And this has paid itself of several times over.

When we write code it’s important we do consider how it may be utilised in the future. Since we cannot make exact predictions it’s better to make methods as small as possible instead to reduce the amount of time spent on refactoring (since this is unavoidable). This also helps us to create solid, more future proof, tests for our business logic.

You don’t need to follow a strict set of design rules, but general guidelines is a good idea. Like trying to follow SRP, avoiding more than x nubmer of lines for your method bodies and trying to avoid nestled code.

My favorite article on this topic is “semantic compression” [0] by Casey Muratori.

0: https://caseymuratori.com/blog_0015

Experience and domain knowledge also matters when designing. For some parts, futuristic thinking is not at all needed while other parts may need it. If you are building a ticketing software, you should expect to add different kind of printers (dot matrix, thermal, inkjet, etc.) so you add an abstraction. The ticket text format may change, so some abstraction will help. The API service hander might not change at all, so an abstraction is unnecessary in that case.

History tells us that we cannot find the right balance until we go through the wrong balance, so is the case with engineering anything.

Learn, practice, and teach to understand what good enough-engineering is.

> public void RemoveItems(Func<Item, bool> condition)

Tangential, but remember that you can't generally push this condition over a database connection. You've just forced your code to be best-case O(n).

Using structured queries as far as possible is useful for performance. But consider what happens if you create a Predicate structure, and someone adds a field without updating the Remove function to match... Perhaps a recursive discriminated union is the way to go, for languages that fail compilation when not all branches are covered? Positional arguments in other languages?

Why does introducing interfaces and vtables have to be the right abstraction? Given the example, the simplest solution is best. If you don’t need runtime semantics, just write simple code.

The Cons of a "simple" solution: > As you add features, the class becomes more cluttered. But if it's a feature, it shouldn't be considered clutter. Clutter usually comes from over-engineering, not necessary additions.

> Each method does one specific thing. That seems fine until you realize your interface is full of shallow, one-off methods. That makes it hard to maintain and extend. However, each method is efficient and tailored to solve a specific feature, which boosts performance. Plus, if there's a bug, it's easy to find and fix since each method has a clear path.

Also, to maintain a clear designed API, I suggest following some data-oriented recommendations, like designing functions to take lists of elements instead of single elements.

For the Vehicle example the conclusion wasn't that the Vehicle abstraction was premature, it was to prefer Composition and Interfaces over Inheritance which is a well discussed idea around OO design.

The Vehicle example is relatable to day to day work but for me the Vehicle abstraction seems reasonable. The mistake is not that a Bike is or isn't a vehicle but rather that the abstractions should align with the business usage: a bicycle doesn't share much in common with a car, the company just wants to rent them out.

So most the stuff that they will share in common will be around the sales/rental part of the business logic. You wouldn't expect them to really be displayed together anywhere either. So I wouldn't be trying to force a Bike into a Vehicle, I'd be looking for the seams related to the truly common operations on Bikes vs Cars and look to extract interfaces or extend polymorphism there

I think this is the common understanding of what 'abstraction' and 'generality' mean in the over-engineering sense, and I think it's wrong.

You make things more general/abstract/future-proof by removing things, not adding things.

ShoppingCart is a collection. We have libraries and libraries of code which work over collections. So List<Item> is better.

Once you start hacking on the various ShoppingCart-related methods a bit more, you'll probably realise you don't depend on Item. Many of those methods should start operating on List<A> instead. And then chances are they'll be pass-through methods that just forward the behaviour to some List implementation, at which point you can delete the passthrough methods, and directly call the List methods directly.

The vehicle example doesn't make sense (start engine? At most, a notification from a rented vehicle to the server that the engine has been started, which is simply not going to arrive from a bicycle).

The shopping cart example is dangerous because it looks reasonable. Actually, the removal methods define atomic operations, so they are a big deal from an architectural point of view: can the cart change while we are testing elements for removal? Can the predicates look at other elements of the cart? Self-contained special case operations might be a more correct choice after all.

If anyone with more than six months experience submitted that vehicle PR with a half dozen interfaces we'd be having a chat outside of the review because that's pretty bad. That might be the worst code in the entire article, and it's supposed to be one of the good examples? This is even ignoring the fact that there are languages that don't support multiple interface inheritance which isn't that big of a deal but hints that the author may not have a ton of experience outside the couple languages he works in every day.

A structural engineer has to get it right the first time, or people may get hurt. Nobody is going to lose their life if a SWE has some inefficient code duplication in their POC that is tested by a maximum of 5 people. There is a time for improving your work and making it more robust, reliable, and efficient — but that can often be done iteratively. Code is malleable, just don’t let it fester and rot forever if you continue to build on top of it, or you will be drowning in tech debt.

I always try to find the unspoken assumptions in these kind of articles. In this case, the need to use classes for everything, as the only hammer.

Pure functions with clearly typed inputs and outputs: you list all the available types and add when you need more supported, like data. They could be in a static class for convenience, or simply in a module.

or some finite state machine or a data-driven approach would make most of the examples much clearer.

It's simple: understand what the goals of your team are for the current year. Write the code with supporting those goals in mind.

It's quite a different thing when you think about just plain code that would need refactoring and data systems that need migrating. Probably an easier decision to keep code more focused on the present than data architecture, because rewriting code, while annoying, is a lot simpler than switching out data systems and migrating data in production.

Re: the shopping cart example

> This approach is good for now. But, it will limit you later. Your code will quickly get out of control.

Will it? If we need to be able to “delete via X”, it sure feels more maintainable to have this feature supported via an explicit, named method vs. inlining it as an anonymous closure (as recommended by the “balanced” solution).

I wish there were a big list of example situations like these to pour over. One could write a whole book on this topic (maybe someone already has). I spend so much time thinking about design decisions like these, but rarely get to hear someone else's thought process.

problems created by being stuck in too much of an OOP / Clean Code mindset.

if your initial entity is Map like - some call them records, data classes, objects etc - you can simply add / remove properties willy nilly. and have functions that filter on those properties.

Holy strawmans, Batman! This article sets up a false dichotomy (virtually nobody argues for designing for "future needs", it's more like varying degrees and definitions of "cleanliness"), then gives examples of terrible code at both ends of this dichotomy, and settles it with very mediocre code in the middle. IExternalRuleService? Is that supposed to resemble anything anyone sane would ever do? Do people literally write "Car : Vehicle" out of their 1st year of college anymore?

Let's assume we're all past those strawman examples and talk about the proposed "Balanced Approaches" instead.

ShoppingCart looks to be re-writing an in-memory collection. Is it actually adding anything beyond "Collection<Item>"? (Substitute with whatever collection type you like in your language). Do you actually need a class at all? There simply isn't enough specified in the article to tell. Therefore there's no lesson here: the code as-is shouldn't exist, and we can't tell what ShoppingCart is actually supposed to be doing. If it's making database calls, you have multiple major problems. The best advice I can give in that situation is: don't do, just plan. RemoveItems should take something that can be interpreted into SQL (or whatever's appropriate for the persistence you're using) and return a plan that can be composed with other plans.

IRefuelable, IParkable, IEngineOperable ... these seem to be missing the point that interfaces are defined by their consumers, not by their implementations. You write an interface when you have a need for its methods, not when you happen to have two implementations of them. If you find you're writing classes like RealLifeObject : IFoo, IBar, IBaz, IBing, IBong, then your class is capturing the wrong thing. Most likely, those Foos and Bars are the real domain objects you care about, and Car should never have been a class. Go read Eric Lippert's excellent Wizards and Warriors (all five posts): https://ericlippert.com/2015/04/27/wizards-and-warriors-part...

It's like this any time I see an argument about over-engineering, abstraction, etc. It just feels like everyone is missing the whole point. We argue about abstraction without even agreeing on what abstraction actually is. (Spoiler: it's many different things.) We get bogged down in silly underspecified toy examples. So many articles are "stop smearing shit all over your face! Smear it on your LEGS instead! Much better!" Why are we smearing all this shit in the first place? It's very hard to have an internet conversation about this.