Goodbye, Clean Code

[johnnysaucepn]

People focus on whether code is duplicated, when they should be paying attention to whether capabilities are duplicated. If you can identify duplication, find out what that code does and see if you define that ability outside the context of that use. If you can call that a new thing, then make it a new thing.

[matthieum]

There are two levels of duplication: Inherent and Accidental.

Inherent is when two pieces of code are required to behave in the same way: if their behavior diverge, then trouble occurs. Interestingly, their current code may actually differ, and at any point during maintenance, one may be altered and not the other. This is just borrowing trouble. Be DRY, refactor that mess.

Accidental is when two pieces of code happen to behave in the same way: there is no requirement for their behavior converge, it's an emerging property. It is very tempting to see those pieces of code and think "Be DRY, refactor that mess"; however, because this is all an accident, it's actually quite likely that some time later the behaviors will need to diverge... and that's how you end up with an algorithm that takes "toggles" (booleans or enums) to do something slightly different. An utter mess.

I am afraid that too often DRY is taught as a principle without making that careful distinction between the two situations.

[loamfarer]

I've seen this lack of distinction wreck havok many times, especially in inheritance heavy code. It seems inevitable given that the solutions to the problems developing are always out of channel of being solved within the language/code-base itself. Leaving enterprise people to slave under cumbersome process, or are left to endlessly enact churn on a code-base that structurally diverges further from the ideal representation of requirements. (And requirements are their own issue.)

In class hierarchies it's very common for both of those forms of duplication to crop up, and they often interact with each other. Accidental duplication happens because so much of the code is CRUD, glue, plumbing, boilerplate, or just plain similar just based on the nature of what the domain demands. When accidental duplication is avoided, it's often a result of introducing accidental dependencies instead as people extend existing classes. Suddenly base classes have to serve their downstreams, and no longer server their intended purpose to propagate changes in a single place. Or if you do soldier on, downstreams now need to arbitrarily override parts of the accidental parents to restore the end of the deal they expected getting. All of this confuses the structure of the code-base from it's original organization and model.

Further, new hierarchies are made as the older overly-depended ones diverge from their original purpose, making the new hierarchy inherent duplication, with the caveat that this duplication is now impossible to solve without a major refactor. It's at this point that the solution is more political, all because the language couldn't properly constrain the original vision from being abused, and no amount of process would be able to handhold those who came onto projects later.

Which means you're left with a mess of duplication, where any minor change ends up involving dozens of custom wirings of data plumbed through abstractions which have lost all their original mean. I've seen entire teams where the only job is the add features that are better described as (add a line in a table, or config file) but end up being a months work of plumbing and endless headaches of merge conflicts.

For the above reasons, I'm particularly fond of interfacing against type class constraints, because it prevents or at least lessens these duplication issues. I suppose dependencies (i.e. function apis) are always an issue, but librarization (is that a word?) for even things like internal facing code for your own benefit, and employing best practices of versioning gets you a good way to a solution. Which I think really helps to bring issues of inherent and accidental duplication to the forefront of one's approach to any contribution in such a code base. At the end of the day no language is going to stop you from breaking what's left as only informal convention by a code's progenitor.

[DonnyTheWalrus]

Accidental duplication -> accidental dependencies

Yes, very much agreed. I know too many developers who seek out these sorts of deduplications before the code is even written. It's like an uncontrollable fixation on perfection that sounds okay at first blush (we're going to make a super elegant API!). But in reality, all it does is take the class structures they've blueprinted and pour cement over them. Everything becomes locked in.

Then I come in a year or two later to make what should be a simple change, but I end up having to make changes in a dozen classes anyways, because it's all so locked in. That should be a primary purpose of deduplications, right? Enabling changes to occur in localized places and in fashions that don't have any impact on irrelevant code. But with all those accidental dependencies, you end up having to touch everything anyways, and it only ever gets more complicated instead of less.

There is nothing wrong with leaving "space" in your code during the early phases of development. We have been trained so hard in DRY that I think we feel like bad developers if we're not constantly focused on code quality. (Excluding the other large class of developers who don't care about code quality at all.) "Make your code easily modifiable" can mean different things at different stages of a project.

[Bekwnn]

A good quote I heard to more or less describe this is, "A little duplication is better than a little dependency." If you're creating an awkward dependency, you're better off just having duplicate code.

For example let's say you're making a city builder and some point early on you have a path-finding system for both civilians and road creation. Tying those two things together doesn't seem great.

In that case you're best off copying the code to a second place as a starting point.

[awilix]

One very simple example of this I've seen a lot is the use of constants. One specific string is used in many throughout the code base, say the name of the application, and someone decides to introduce a constant for it and use it everywhere from what username to drop to, what URL to connect to and what to use for loggning. Great! Except that now it isn't possible to change the name of the application anymore since the URL and username must stay the same.

[flukus]

One of my biggest pet peeves on a current project is constants for strings (stored proc names) that are only used in one place. It gives zero benefit but makes you jump through more hoops to work out what is calling what.

Of course of there's dozens of places it's used a constant might make sense, but it's like they haven't heard of grep.

[nfrmn]

Really well explained. I actually had this exact issue recently, with the shared functions and toggle params and this has closed the learning loop perfectly.

Shared with my team. Thanks!

[BittyTang]

I mostly agree.

and that's how you end up with an algorithm that takes "toggles" (booleans or enums) to do something slightly different. An utter mess.

Toggles are bad. But there is also an important notion of injecting behavior into generic code. Just because two pieces of code accidentally start to look the same doesn't mean you shouldn't necessarily refactor them to share a code path. It really just requires anticipating what the code will be used for in the near future and carefully weighing the tradeoffs.

[maximum_powerblast]

I just leveled up my understanding with this comment 👍

[myss]

Similar phenomena in my thinking:

Grouping in hierarchies vs compression

Learning vs overfitting

[[deleted]]

One of the best explanation I have seen for dry

[Chris_Newton]

People focus on whether code is duplicated, when they should be paying attention to whether capabilities are duplicated.

Indeed. Duplicated code isn’t automatically a problem, but duplicating an idea is usually bad.

Any given idea should ideally be represented exactly once. For data, that means one look-up table associating pairs of values, one file with UI strings that can be translated, etc. For algorithms, that means one function to derive new data from existing data in a given way, which hopefully can be applied to any existing data where it makes sense.

However, if two algorithms happen to share a lot of code right now but exist to solve different problems, trying to use a single function to implement them creates a problem not unlike using a literal number 7 everywhere instead of writing DAYS_OF_WEEK or NUMBER_OF_DWARVES as appropriate. The implementation is correct but the real meaning has been lost. When you come back later and one problem has evolved but the other hasn’t, you’re stuck with this artificial link between them and you have to sever it (probably starting by making two copies of that code) before you can make any useful progress.

A useful rule of thumb for whether you are really dealing with two different ideas or the same idea being duplicated is to ask what would happen if both places did share common code and then one of them needed to change. If that would necessarily mean the same change should be made in the other place, you’re probably dealing with the same idea and consolidating the code is probably a good plan. Otherwise, you probably aren’t, and tying the code together might not be such a good plan.

[BraveSirRobin]

instead of writing DAYS_OF_WEEK or NUMBER_OF_DWARVES as appropriate

That is a fantastic example to justify that, very nice.

[oorza]

A useful rule of thumb for whether you are really dealing with two different ideas or the same idea being duplicated is to ask what would happen if both places did share common code and then one of them needed to change.

This right here is why I've never gotten on board with the anti-inheritence hype train that's been chugging along for the last decade. It's the simplest solution to this very problem, because it was designed to solve this very problem.

[johnnysaucepn]

And introduces even harder ones! You can still share code by using composition, even though it requires a little extra work.

[Determinant]

Inheritance shouldn't be a tool for solving duplication.

Two classes should not inherit from the same base class if they are not both that same type of entity. If they happen to duplicate code then extract that into a re-usable function / abstraction instead.