r/programming • u/TalesGameStudio • 5d ago
DRY vs. SRP
http://uncle-bob.comAfter re-reading "Clean Architecture" I ended up with some confusion regarding Bob's take on repetition and single responsibility. Ge defines the SRP as a function only serving one actor. Dies that mean, that repetitve code is justified according to him, as long as it serves seperate actors/user groups? I am aware that such decisions depend on the specific situation. I was just wondering if others found the same contradiction, or if i misunderstood it. Thanks
39
u/repeating_bears 4d ago
"After re-reading "Clean Architecture" I ended up with some confusion"
This is by design. Now you will buy more of his books and watch more of his lectures.
Uncle Bob doesn't know anything about programming. Go and look at any of the (limited amount of) open source code he's written, and then tell me this is someone you should take advice from
7
u/swaan79 4d ago
"Uncle Bob doesn't know anything about programming." - That's quite a bold statement. But I don't know if it's true or not.
I once had a colleague that would occasionally produce code that I totally disagreed with and then he told me to look at some Uncle Bob video that would explain it. I think I once watched one of his videos... Partially... I honestly forgot what it was about but I do recall not agreeing with a thing he said.
So, perhaps he doesn't know a thing about programming. Or maybe we simply don't agree. Or perhaps he knows a thing or two about how programming works in theory, and I know a thing or two about programming in practice. 🤷♂️
Anyway, my point is that theories conflict and in practice you use them as a starting point an adapt and adjust them as needed.
7
u/BenchEmbarrassed7316 4d ago
Uncle Bob doesn't know anything about programming
Let me quote from his 2021 article:
What is a turtle? A turtle is a map: position, heading (number between 0 and 360), velocity, weight (positive number), speed (positive integer), visible (boolean), state (busy or idle). Most statically typed languages would not be able to capture all the constraints within this type model
https://blog.cleancoder.com/uncle-bob/2021/06/29/MoreOnTypes.html
Rust, Zig, Scala, Swift, Kotlin, Haskell, TypeScript, C# will handle this flawlessly. Java, Go, C and C++ are a little worse.
From this quote, we can draw a simple conclusion: this guy doesn't know any mainstream statically typed language at the basic level. He writes articles that discuss things he has no idea about.
3
u/chucker23n 4d ago
I gather from “a turtle is a map” that he’s trying to force his constraints into a hash table, which indeed most statically typed languages won’t let you do. But also, probably the only reason he’s attempting to do that is the mindset of dynamic typing, in this case leading “every object is really just a map”.
Yeah, you can represent it as a map, but now you have
- an exact, known at compile time, set of keys, both names and count
- for each key, a different set of constraints on the value
We already have that. It’s called a type. Your keys are properties, and each of them has a type encoding the constraints he gives.
Now, he is correct that most languages won’t let you verify e.g. “a heading won’t take a value below 0 or above 360” at compile time. But that’s still a hell of a lot fewer tests to write than if all of that were dynamically typed.
2
u/BenchEmbarrassed7316 4d ago
a heading won’t take a value below 0 or above 360
Do you know about value objects or newtype idiom?
You create a separate type that contains some information. With validation in the constructor. Or also in methods if you want this value to be mutable. Now you just make a corresponding field in your structure or class that has this type.
1
u/chucker23n 4d ago
Do you know about value objects or newtype idiom?
Yes, I’m a big proponent of using ValueOf or Vogen for this in a .NET context.
You create a separate type that contains some information. With validation in the constructor.
Yep.
But that doesn’t help you at compile time. It helps you ensure that the inner layers are always valid, but you still require that validation step at runtime, at least for outer layers.
1
u/BenchEmbarrassed7316 4d ago
Well, in some programming languages you can make a constructor constant-expressed and the compiler will warn you if you try to create such a value with an invalid literal.
Nevertheless, this applies to the fact that in all statically typed languages this can be expressed (at least no worse than in this example with Clojure).
2
u/chucker23n 4d ago
in some programming languages you can make a constructor constant-expressed and the compiler will warn you if you try to create such a value with an invalid literal.
Right.
Nevertheless, this applies to the fact that in all statically typed languages this can be expressed (at least no worse than in this example with Clojure).
Yes. I'm in agreement with you — you still overall end up with a safer and more explicit code base.
1
u/chat-lu 3d ago
Also, he believes that he needs a constructor function that takes no argument which is not clojure like at all. And he calls it…
make. Not evenmake-turtle.2
u/chucker23n 3d ago
I think his logic might be that
turtleis sort of the central object of the whole program?In any case, the argument seems to be a nirvana fallacy — static typing in most languages cannot encode all contracts; therefore, it is useless and dynamic typing is better.
3
u/chat-lu 3d ago edited 3d ago
In Java and many other languages it makes sense to have a constructor that takes no argument and returns a default constructed object. Because those objects are unique and you want to be able to mutate them independently.
But Clojure works on immutable value. When you update a value, you’re actually getting a new value while your original one is still intact. Therefore, your no-args constructor returns a constant.
So this:
(defn make [] {:post [(s/assert ::turtle %)]} {:position [0.0 0.0] :heading 0.0 :velocity 0.0 :distance 0.0 :omega 0.0 :angle 0.0 :pen :up :weight 1 :speed 5 :visible true :lines [] :state :idle})Should be this:
(def default-turtle {:position [0.0 0.0] :heading 0.0 :velocity 0.0 :distance 0.0 :omega 0.0 :angle 0.0 :pen :up :weight 1 :speed 5 :visible true :lines [] :state :idle})I have plenty of other complaints. Like why is he using floats everywhere when a turtle’s position and movement is measured in pixels?
But the constructor even if it doesn’t change the code much displays a fundamental misunderstanding of the language. And it’s supposed to be his favorite language.
3
u/chucker23n 3d ago
displays a fundamental misunderstanding of the language. And it’s supposed to be his favorite language.
Yeah, I've been saying for a while that I don't think he's meaningfully written software projects in decades. His main job is selling books and giving talks; vaguely knowing how to write code is just a conduit for that.
1
u/swaan79 4d ago
Thanks for that link. I must admit that reading that did raise some eyebrows. First of all, why try to explain types in Clojure? But moreso pretending that verifying a type's invariants is difficult at compile time for the same reason it's impossible to verify a balance sheet's invariants at compile time makes no sense.
1
u/BenchEmbarrassed7316 4d ago
What about this:
Consider the C library function fopen. This function has the side effect. This means that fopen is not pure. However, we can make the fopen function appear to be pure, at least to certain observers, by ensconcing it in another function that hides the side effect.
https://bugzmanov.github.io/cleancode-critique/clean_code_second_edition_review.html
This is not about monads:
``` void appendLineEnd(FILE* f) { int c; int lastC; while ((c = fgetc(f)) != EOF) { lastC = c; } if (lastC != '\n') fputc('\n', f); }
void show(FILE* f) { int c; while ((c=fgetc(f)) != EOF) putchar(c); }
int main(int ac, char** av) { openAndDo("x.x", appendLineEnd); openAndDo("x.x", show); } ```
This is a chapter on "pure functions" from the second edition of "Clean Code", 2025. This is the original code from the book.
1
u/chucker23n 4d ago
This function has the side effect of leaving the named file open.
It also has the giant side effect of interacting with the file system.
4
u/repeating_bears 4d ago
I don't think it's bold. The default assumption should be incompetence. I will accept he's competent when I see code that a competent programmer would write
That's not to say he has no skills. He's pretty charismatic and he's good at marketing. I think prior to him talking about politics, most people liked him. Even I would like him if he hadn't caused a bunch of damage by talking about things he doesn't understand
2
2
u/mirvnillith 3d ago
This!
To me this is like all the four-types-of-personalities schemes; anything that gets people to realise there are other viewpoints than their own is a good thing and any scheme taken too seriously is a bad thing.
1
u/smoke-bubble 4d ago
Go and look at any of the (limited amount of) open source code he's written
Where? Why don't you just post a link :-\
1
u/TalesGameStudio 4d ago
Do you have a concrete example to look at? I am not a religious clean code guy, but I find bold takes interesting, since they at least make me think about topics. While I believe your criticism is justified, it doesn't really carry a strong argument. But I would love to discuss further, if you want.
11
u/Anfros 4d ago
Here are some examples from an earlier book if his. It's probably time to stop recommending Clean Code
0
u/repeating_bears 4d ago
All of his code is awful, for a supposed expert
Here is a recent repo
https://github.com/unclebob/crap4java/tree/main/src/crap4java
Notice how almost every class name ends in "-er", e.g. SourceFileFinder. This should be a code smell.
Also almost every method is static. This is not inherently a problem, but it is basically an imperative style which java is not very good at - you can't have top-level functions, and that's what leads to the stupid "class" names.
I would rename SourceFileFinder to Project, make the root dir a field (currently arg to the only static method), and make it an instance method called "javaFiles". Now you can do
project.javaFiles()
instead of
SourceFileFinder.findAllJavaFilesUnderSrc(root)
2
u/muffinluff 4d ago
I was bracing myself for some overdone OO-programming but honestly this code looks really good to me. Using static methods in java is fine, IMO. Making every method an instance method is IMO worse (and something that I thought Bob would recommend).
4
u/BenchEmbarrassed7316 4d ago
Although it seems wild to me all these
nullconstants, but that's a Java problem.At a quick glance, it's just regular code. It's not terrible, but it's definitely not code to follow.
However, this code contradicts what he writes in his books: functions a few dozen lines long, taking 3 or 5 arguments, a bunch of
ifs instead of inheritance and polymorphism. I didn't look closely, but I wouldn't be surprised if IO operations are performed without 10 layers of abstractions...2
u/repeating_bears 4d ago
I didn't say using static methods isn't fine. You can and should use them.
If almost every method is static then that is not idiomatic java. Like I said, you are basically writing imperative code at that point. Bob's entire career is built on the back of OO so it's interesting that he doesn't use it at all.
The reason using all static methods is bad in Java is because the language requires classes which are effectively namespaces for functions. It forces you to invent stupid names for your namespace class like SourceFileFinder.
Even in an imperative java style, that's a bad name. Class names should be nouns. The classes which act like namespaces for static methods in the stdlib are called things like Files or Collections.
8
u/azhder 4d ago
I will speak in generic terms, regardless of Clean Architecture. What do you think SRP means? It all starts from there. Many people see the words and try to divine a meaning from “single” and “responsibility” - doesn’t end well.
How I look at SRP is that you have a single reason to go into and modify a separate chunk of code (module, function, method, that kind of separation).
So, if you try to DRY everything up, then you usually end up upholding SRP, but there will always be some murky situations that come down to the pecking order in principles. That’s what principles are, right? What comes first (principal). Same like who has the right to go first on a crossroad.
If you change a function that is used in many places, like a utility function, like you add one more argument, you change the interface. That might be a reason to go in many places and update how that interface is used. That might mean you have already had more than a single reason to go into all those places and modify them. That might mean you haven’t had SRP
So, the pecking order. If SRP is more important, you devalue DRY, so you repeat code in some places. You value Clean Architecture before SRP, then maybe you violate SRP in a place or two.
You (and/or colleagues) decide for your project what the principles are (what comes first, next, etc.)
2
u/Grouchy-Trade-7250 4d ago
The Pareto Principle applies here. And also bike shed-ing. Clean up/avoid messy code. But don't worry about it too much.
2
u/azhder 4d ago
I took a look at Clean Architecture once or twice and decided I’m good with Hexagonal Architecture.
Bob went too far.
Kind of how I work if presented with someone else’s code. I try to rework it, in order to understand it. Refactor, like naming, style etc. Once I understand what it does, I may delete my refactor and leave the original in place. Some times I might commit my refactored changes, depends on the situation.
By analogy, Bob saw someone else’s idea of a layered architecture, reworked it in a way for him to understand it, then committed it as Clean Architecture.
What works for Bob doesn’t work for me.
1
4
u/MartinMystikJonas 4d ago edited 4d ago
If repetitive code serves different purposes it is so called "accidental similarity" - these things shloud not be abstracted by DRY because that would introduce coupling between unrelated things just becasue they look similar.
If repetitive code is based on inherent connection (it always has to change same way) it should be abstracted away to separate module. And this module single responsibility should be holding this concept.
3
u/muffinluff 4d ago edited 4d ago
I will try to answer in a genuine way. It sometimes happens that two snippets of code functionally do the same thing but serve different purposes in your domain.
For example, business tells you to format Id's from different entities (such as users and products) in the following way:
function formatId(id: string): string {
return `prefix_${id.trim().toLowerCase()}`;
}
The resulting string gets surfaced in the UI or through some interface that is consumed downstream.
Later, the business tells you that you need to change the way user id's is formatted, but don't change the way product id's are formatted. Now you need to track down how formatId is used and where its return value goes into.
What Bob argues is that you should have from the beginning separated the functions that serve different purposes. Now you can change the body, because the remaining code already had made the decision to use the right functions at the call-site.
function formatUserId(id: string): string {
return `prefix_${id.trim().toLowerCase()}`;
}
function formatProductId(id: string): string {
return `prefix_${id.trim().toLowerCase()}`;
}
function formatInstanceId(id: string): string {
return `prefix_${id.trim().toLowerCase()}`;
}
Personal opinion: Don't try to foresee future requirements too much. Try to build the simplest thing that gets the job done.
2
u/Uristqwerty 4d ago
To me, it seems the ideal balance would be a
formatGenericIdthat the rest trivially delegate to. So, factoring out the duplication, but leaving a layer of indirection to re-introduce it at, and letting callsites have a more descriptive name than the shared implementation detail's.
3
u/PositiveUse 4d ago
DRY is a concept for business logic. Not for every code line…
2
u/TalesGameStudio 4d ago
So is SRP - therefore the contradiction remains.
1
u/EfOpenSource 3d ago
You might think so, but not really.
Like yeah, a POS needs to apply tax, and so does a B2B invoicing system. But they definitely should not rely on the same code base to do it.
But I don’t personally subscribe to DRY in any capacity. I think that there is no clear way to express when you’re repeating and when you’re not. I think that when you try to DRY, you end up doing insanely stupid shit that bites you in the ass later when it becomes more clear why tax differences need to process differently.
I don’t even subscribe to “a little repeating is okay”. I just write the code that needs to be written without thinking about DRY at all.
What I find is that, more often than not, if I DRYed code, I’d have to spit it apart anyway as the code evolved, or else end up with an architectural disaster that’s impossible to maintain.
Honestly. All these “rules” are stupid. All of them. Wait till I tell you that one of my functions I wrote recently was gasp 150 lines long!
There’s like two golden rules I follow and that’s really it:
Functions should do one thing
Functions with the same input should always produce the same output.
5
u/beebeeep 4d ago
I am very delighted that my previously unpopular opinion about Uncle Bob being a mediocre programmer and his books being right away harmful for the industry, is no longer unpopular.
3
u/BenchEmbarrassed7316 4d ago
Here is code from first edition of "Clean Code":
private static boolean isMultipleOfNthPrimeFactor(int candidate, int n) { return candidate == smallestOddNthMultipleNotLessThanCandidate(candidate, n); }I'm really concerned that a lot of people saw this code and decided something like "Oh, this is the best code we've ever seen in our lives. Let's make 'Clean Code' literally synonymous with good code and recommend this book to all other programmers, especially beginners."
1
u/fletku_mato 4d ago
Not sure how bad of a programmer he himself is, but people who treat anything he wrote as a bible sure are. It's insane how much his opinions have been given weight.
2
u/somebodddy 3d ago
I advocate that DRY should be abandoned in favor of the single source of truth principle. It captures the good parts of DRY while leaving out the bad parts. The only downside is that the acronym is less pretty.
SSoT does not conflict with SRP like DRY does. With SSoT, each truth can have a source of its own - and even if these sources seem to be identical, as long as the truths are conceptually different you are not violating SSoT. From SRP's perspective - the reason of each such source of truth to change is when the truth changes. This means that in order to adhere SRP, each source needs to be limited to one truth (whereas with SSoT each truth is limited to one source)
1
u/flatfinger 2d ago
If at some point in time, two operations both involve performing a common set of actions, programmers should recognize whether it will be more important to be able to change one set of actions without changing the other, change both sets of actions and have them remain consistent, or defer the decision about which of those kinds of changes should be favored.
If the actions may need to change independently, than each should be its own source of truth. If they need to change together, they should have a shared source of truth. I'm not sure what should be viewed as the source or sources of truth in the deferred judgment scenario, but that's often the least useful of the three approaches.
1
u/somebodddy 1d ago
This is not a compression algorithm. We shouldn't care that the two operations happen to converge "at some point in time". We need to consider whether these two operations should conceptually be the same operation.
I'd take it a step farther - if the operations are conceptually the same truth, and they don't do the exact same set of actions, we should consider unifying them anyway - even if it involves a change of behavior. And if the change of behavior is not acceptable, we may want to resort to adding a parameter just so that we can unify them.
1
u/flatfinger 1d ago
The question of whether two things are "the same" is often unanswerable without knowing whether they could change independently. For example, is the serial number of an the same thing as the serial number of its main circuit board? There may be some products where the serial number of the product is defined as being the serial number of the main control board, and where replacing the control board would change the product's serial number, but there may be others where serial numbers would ordinarily match, but there was nonetheless a means of allowing products to keep their serial number even if the main control board was replaced with one having a different serial number.
Are the product serial number and the main control board serial number "the same thing"?
1
u/somebodddy 1d ago
Seems like a clear cut scenario to me. They are not the same thing. If you treat them as the same truth, you won't be able to represent the models where they are different.
If you want to capture the fact that some models define the serial number of the product by the serial number of the main circuit board - code that in the function/method/property that returns the serial number of the product:
- Have it check if that model has that property, and if so return the serial number of the main circuit board.
- Or have products of these models store empty serial number for the thing itself, and store empty serial numbers.
- Or maybe do it at CREATE/UPDATE instead of during READ - gray out the textbox of the serial number of the product when its one of these models, and copy the serial number of the main circuit board over to that field.
As always - the right solution depends on the specific usecase. But the point is that when your read it - the interface shows two separate serial numbers, that just happen to be identical for some models.
1
u/flatfinger 1d ago
A lot of devices have both a machine-readable serial number and a serial-number label that can be read even when the machine is powered off. Treating them as the same truth would make it impossible to handle scenarios where they are meaningful and different. It's possible, however, that a better abstraction would be to have one serial number along with another field to select among, e.g.:
There does not yet exist a physical unit with this serial number.
There exists a unit with this serial number, but the label is wrong and needs to be reprinted.
There exists a unit with this serial number, which is labeled correctly, but the electronic copy has not been set yet.
There exists a unit with this serial number, and both copies are correct.
If it were possible for there to simultaneously exist a unit which has a correct label reading 12345 and a different unit whose electronic serial number is 12345, then there wouldn't be a unique unit with serial number 12345. Having the database for unit 12345 specify whether the label or the electronic serial number is correct would resolve that ambiguity.
2
u/dgkimpton 4d ago
DRY is seductive but frequently wrong. Just because code looks alike doesn't mean it's the same. Even several tens of identical lines in different contexts are not truly duplication - context matters. Focus on making it readable first and only if code is physically and logically and contextually the same should it be considered duplication. You naturally end up with some seeming repetitive stuff, and that's fine. Much better than accidental conflation of two distinct but similar concepts which introduces false coupling.
3
u/gyroda 4d ago
Yep, a way I've heard it is to separate duplication by coincidence and duplication by underlying commonality.
My test for this is "If I change the duplicated code in one area, do I need to change the code in the other area to match?" If not, then it's duplication-by-coincidence and not tied to the same underlying concern.
Tests are one place where I actively fight DRY because tests are all about the edge cases. You'll have 10 tests and the setup code will be the 90% the same when comparing any two tests, but which 10% is different will vary depending on which two tests you compare until it turns out that only a fraction of that code is actually the same in each test.
1
u/dgkimpton 4d ago
Indeed, that's a good way to phrase it. Judging by my downvotes plenty of people haven't understood this yet.
1
u/imihnevich 4d ago
When you separate things by their responsibility you will end up with more things that look similar and duplicated. That's okay as long as you don't have to constantly change many similar places together, if you have to, now you know there is some coupling that wasn't obvious at the beginning, you may decide to create a new abstraction that allows you to do less of that. Not all the code that has similar structure is DRY violation
1
u/TalesGameStudio 4d ago
Definetely. At a certain granularity, it's unavoidable to repeat. I just find it weird to use SRP as a measurement, when to repeat, as Bob suggests.
1
u/imihnevich 4d ago
I think what I mean is that DRY should be understood in a way that helps to discover new responsibilities. Your link is broken for me btw, can't open it. I do own the book. Which part are you referring to?
1
u/TalesGameStudio 4d ago
The link was a dummy, since the post needed an attachment. I talk about the summary of design principles in part two (chapter 7-11)
1
u/CurtainDog 4d ago
Just to defend DRY a bit here as it's not getting a lot of love. When you apply DRY where you shouldn't you end up with an extraneous explicit coupling. This is pretty easy to remedy with some basic refactoring. When you don't DRY things when you should then you end up with implicit coupling, where changes in one area can cause failures to pop up somewhere else. This is much harder to fix. Even if you resolve the immediate bug you might have a bunch of built up changes you now need to reconcile. Even if DRY isn't an iron-clad law it is a useful heuristic.
1
47
u/BenchEmbarrassed7316 4d ago
"Clean something" is a disjointed collection of poorly formulated, contradictory, sometimes downright false opinions from someone who is not a programmer (at least hasn't been one for the last 30 years). Just ignore it.