On DRY and the cost of wrongful abstractions
thereignn.ghost.io
thereignn.ghost.io
1- Do no generalize so you can factor out like code too early. The first time just write it. The second time write it again even if only minimally different. The third time is a judgement call. By the fourth time you see the proper abstraction, something that might have been much different if you were to have refactored early on.
Corollary to #1- Don't run ahead of yourself making all these placeholder functions, interfaces, and abstract classes with the expectation that you would have had to refactor this way later. Unless you done versions of your project a dozen times (even then, sometimes I don't even do it), you are most likely going to be wrong.
2- Don't refactor out single lines unless there a lot going on on that line or you have the experience to know (don't lie to yourself either) that this will be an important and growing line.
3- Sometimes performance can trump. And sometimes you do know this ahead of time, especially if performance is correctness as it is on some projects with very tight constraints.
Besides that. I freaking hate seeing similar code everywhere, blowing up my instruction cache, making changes difficult. Please stop it.
Generally, the most useful tool in refactoring is to use extract method relentlessly, even into 1-line functions. More often than not, a well named function with well named parameters is one of the absolute best and most reliable ways to make your code as close to "self-documenting" as possible. But there are additional advantages as well.
Spinning things off into function calls often makes refactoring easier. Because it discourages bad habits like local variable reuse (using variables as scratch pads) and excess mutable state. It also encourages a more functional style, which has a lot of benefits especially in multi-threading but also in the ability to reason about and determine the correctness of code. It can also make it a lot easier to see when you need to spin off functionality into other classes/modules. For example, if you have a member field and you have a ton of little one line snippets everywhere that anything happens with that field, that usually means you need to extract it out into a class and have those one-line snippets be various methods on that class. When all that code is inline, this can be harder to see.
Additionally, there are lots of mental blocks that happen when you have inline "one liners". There's a bias to avoid "cluttering them up", even when it's necessary. For example, when you need to add error checking and error handling. When you've spun out the code into a method call, you tend not to have that block, you just add the error handling code because it's trivial, and going from a 1-line method to a 5-line method isn't a big deal. But bloating up another function by taking a sweet one-liner and replacing it with 5-lines of code can seem like clutter, so often times you just have an aversion to doing so. Additionally, sometimes those "sweet one-liners" can be more readable when they're broken up into multiple lines, and this is again something that is trivial and straightforward when it's spun out into its own method but will see resistance when the code is inline. This is true even if the code remains a single statement broken up into multiple lines to enhance readability.
A lot of the time these patterns aren't obvious. Because the end result of spinning off one-liners is often not one-line method definitions, it's short methods that are a couple lines with several lines of extra error detection/handling. Or it's separate utility classes to encapsulate handling of special member fields.
The idea of ensuring that you always have "meaty" functions is unhelpful, what's important is thinking about things from a perspective of abstraction, modularity, readability, and correctness.
I really and truly hate code where people think they need to take the programming out of programming. This goes all the way back to when they learned it was a good idea to comment their code "next = ++i // increment index then assign".
> Spinning things off into function calls often makes refactoring easier.
You're breaking a rule in keeping things simple. Don't anticipate. You are refactoring ahead of the refactoring, most likely making the wrong decision in the process.
And you're also coding defensively against your fellow developers. This tends to be a problem in low- to average-skill workplaces. Instead why you just follow cheakins and show your junior developers what they are doing wrong? That is how I learned to program from some amazing developers. It wasn't from them coding defensively against me, but from the tap on the shoulder I got asking me why I did something on my last checkin.
And I'm not advocating "meaty" functions. They should be divided on logical pieces of functionality that aren't so small where every line qualifies, but not so large as to span more than a page or more do a single thing. Before they even hit the page mark, most likely other reasons to will dominate though like the single responsibility principle or something else.
Me and you might just come from different worlds.
We agree your sample has been obfuscated.
/// <summary>
/// Case Insensitive IRC Name
/// </summary>
public struct CIIrcName {
private string Data;
public static implicit operator CIIrcName( string s ) { return new CIIrcName() { Data=s }; }
public static implicit operator string( CIIrcName n ) { return n.Data; }
public string ToLower() { return (Data??"").ToLowerInvariant().Replace('{','[').Replace('}',']').Replace('|','\\'); } // Silly scandinavians :<
public static bool operator==( CIIrcName lhs, CIIrcName rhs ) { return lhs.ToLower() == rhs.ToLower(); }
public static bool operator!=( CIIrcName lhs, CIIrcName rhs ) { return lhs.ToLower() != rhs.ToLower(); }
public override bool Equals( object obj ) { return obj is CIIrcName && (CIIrcName)obj == this; }
public override int GetHashCode() { return ToLower().GetHashCode(); }
public override string ToString() { return Data; }
}
Most of the changes I'd make here have nothing to do with method length:
1) Conversion (at least back to string) should probably be made explicit at this point (was originally implicit for ease-of-use when replacing string s)
2) ToLower could be made private (only used inside the type) and have the comment replaced with the above blurb (instead of being a comment to myself that only I'll properly decode.)
3) Rename CIIrcName to CaseInsensitiveIrcName?
4) Rename Data to OriginalCaseName?
> And you're also coding defensively against your fellow developers. This tends to be a problem in low- to average-skill workplaces.
My experience has been just the opposite. Low/average skill tends to mean less asserts, less static analysis, ignored warnings instead of warnings-as-errors, no unit tests, no thread safety annotations... forget my coworkers - I add these things to code defensively against future-me, and I've found it super effective on larger codebases.
> This goes all the way back to when they learned it was a good idea to comment their code "next = ++i // increment index then assign".
We agree this is bad too. A different perspective though - those are perfectly reasonable comments when you're new enough to programming that you don't remember what "++i" does! But it's something to grow out of and remove.
But like I've said in all my comments. There are times and places for all these maxims and rules to be broken. That's why programming is difficult, especially that is why finding the right abstraction so that it looks easy is difficult. The best programmers I know always seem to find that right balance and you look at their code and go "I could do that" when in reality you couldn't.
I receive a weird mix of normalized and unnormalized input over the network from servers and software I don't control, and want to keep the unnormalized versions for display purposes.
I could normalize on construction - might save a tiny bit of performance? - but then I'd need to store and keep in sync two separate strings (one for display, one for comparison.) Not that I have any real mutation going on that would violate that invariant in practice.
> so much that you a full class dedicated to them and you don't have the the name normalized ahead of time.
The general pattern is: Parse the network message (containing mixed normalization), then immediately lookup (or create) the target channel/user - allowing no duplicates due to mixed normalization.
With this I can simply construct e.g. Dictionary<CIIrcName,...>s and not need to remember to normalize every time I want to query by a given key. Only about 5 members that reference this type but there's a good 20-30 key lookups/equality comparisons using those members, handling different network messages among other things. That list will probably expand.
I could've wrapped dictionary access instead, but this seemed simpler (as dictionaries aren't the only thing I'm using, and none of the existing code wrapped dictionary access.)
Bingo.
And this is another great reason to have more, smaller methods: it's far better for unit testing. When you have big, chunky function defs it can be a chore to figure out how to reach into the method and test one particular aspect of it. When your code is well composed into functions that are logically consistent, you can write unit tests that target each piece individually.
"Made it so a future modification is made in one place instead of four (possibly missing some)" probably is.
If it were easy enough to give someone a simple checklist of guidelines and have them produce high quality code, coding would be a lot easier, but it doesn't work that way. Coding well requires years of experience that produces the wisdom and perspective to know when and to use different techniques. It's what tells you "it's unnecessary to comment an i++ line", it's what tells you when and how to use one data structure over another, and so on.
And no, I'm not "refactoring ahead of the refactoring", I'm refactoring where it makes sense, even down to a single line. That's the point. To properly compose things, and to avoid the bias against small functions. Not to atomize a program into infinite function calls, of course.
Most of the code I've come across that is below the quality I'd like tends to have excessively large function definitions, precisely because there is such a widespread antipathy against splitting things up "for no reason" and against small functions. When it improves readability, when it improves the composition and modularity of the system, when it makes it easier to update, modify, improve logging and error handling, etc. those are all good reasons for extracting methods. The rare times I've seen code that required excessive jumping around to figure out what anything did it wasn't because of too many small methods, it was because of overzealous application of bad design patterns (e.g. call a factor method to produce a configuration object then instatiate some other factory and pass the configuration object to it to get a factory that produces what you want, that kind of nonsense). That's indicative of a failure of judgment.
My point is, don't be afraid of small functions when they make sense. There are too many people who are afraid of small functions because, like you, they have these strong biases against them based on prejudice. How do you know when a small function is a good idea? Judgment and experience, of course, same as always.
Disagree 100%. But of course as everything else in our industry this is super subjective. I'd much rather a line of combinators which read like English than a collection of function calls with wacky parameter calls. YMMV on this though and I understand that.
I also want tiny functions and those functions to be moved far away. I want the "english" names as close to the Real Work as possible. If I really care what the activity is I can use the IDE to drill in. But again, I understand YMMV on this one.
People who argue this topic as if there's a ground truth are the true problems. Everyone has they're own take.
Exactly. It's like design patterns, algorithms, and data structures. There are lots of techniques useful in programming. The trick is having the experience and judgment to know when to use the right one.
But this depends entirely on the context of the code. I like to think of this in terms of "abstraction levels". Mixing abstraction levels harms readability and so if your code is doing some low level operation then string.append(val) should be obvious from the context and the name of the function it belongs in. If however the code is still at an abstract enough level then you want to wrap your implementation detail in a name that is appropriate for that level of abstraction.
As a rough example, in a function that's popping from some data source and pushing into a local structure as a part of some larger unit of work, wrapping your collection.append(val) would be harming readability. But if you're "enrolling" a "student" into a "class" you should be doing class.enroll(student) even if the implementation is just class.students.append(student_id)
After calling class.enrol(student), I would expect the student to be enrolled, not just added to a list of students.
Why do they have to be different? That's really his point. In this situation, enrolling just requires adding their student id to the list of students in the class. But while doing 'class.students.append(student_id)' accomplishes that, it is not clear reading that line that it actually enrolls the student in the class, rather then just being one step of a larger process, or something else all-together.
As the article mentioned, the thing you spend most of your time doing is reading. Using lots of small functions means you have to bounce all over your codebase to figure out whats actually going on. In procedural languages a complex 'meaty' function often needs to be understood in one conceptual bite. Needing to bounce all over the codebase to figure out what those little functions are actually doing is hugely distracting while I'm trying to read.
I often start those big functions with a block comment explaining what I want the computer to do ("// The overall goal is X, which needs Y and Z to be true. There are 4 cases to consider: A, B, C and D through which these invariants need to be true..."). Then I write the code. The code itself will be delineated with comments calling back to the documentation ("// Case B - the wendigo. Note we're being careful here to make sure that x is always < y"). If I find myself duplicating blocks inside that function, I'll pull those shared blocks out. But single lines? When I'm reading the code later I'm going to want to track exactly what happens through the function at a mechanical level. I want to be able to see the invariants, and track exactly how each value flows through. And for that, the less scrolling and bouncing around my codebase I need to do the better.
Often separate cases are cleanly distinct enough that it makes sense to extract them into their own functions, but sometimes they're not. And thats ok too.
In a sense you want to split things up based on how you expect to read it. If I have lots of tiny functions that are only used once or twice, I'm probably going to forget what they actually do and need to read them to understand the code that calls them. In that case, the code should just be inlined.
Ideally those functions should be named such that their jobs are fairly obvious.
But a call to `setImpulse(spaceship, 0, somevar)` might just set member variables ximpulse and yimpulse, or it might make a bunch of calls to the physics engine, or both, or something else entirely. If I'm trying to understand some code that calls setImpulse I'll probably end up looking it up just to be sure. So in that case, if it does just set the member variables I'd prefer to just write `spaceship.ximpulse = 0; spaceship.yimpulse = somevar;`.
Same thing, but less work to read.
For the first, well named single line functions are great. For the second, they require a lot of jumping around.
Most projects fail badly trying to twist OOP to get there. The code patterns are so predictable, its not even a surprise anymore.
More often than not, a well named function with well named parameters is one of the absolute best and most reliable ways to make your code as close to "self-documenting" as possible.
It might make a single function’s implementation nicely self-documenting, but what really matters is how readable and maintainable the overall code base is.
When we’re reading the code calling that one-liner function, in most languages you can’t see the names of the parameters, only the order in which they are provided. Sometimes the meaning will be obvious anyway, or it won’t matter, for example if you have a commutative function with two parameters. Sometimes the meaning could be made obvious but hasn’t been, for example the infamous boolean parameters where the call winds up being
do_something_with(true, false, true);
Another potential danger with using lots of very small functions is that while each function individually may be nice and clear, the number of relationships between those functions is much greater. This can cause a great deal of frustration to a reader who has to keep jumping to another part of the code to decipher each “clearly named” one-liner function. It can also make it harder to examine the behaviour in context when testing or debugging, for example because log functions don’t have enough information available to produce complete, self-contained messages, or because every time you hit a breakpoint in the debugger you have to walk 6 levels up the call stack to figure out what’s going on at the time.
Finally, I suspect the benefits of one-liners potentially expanding to include error handling are mostly illusory. How often can that one-liner really take any useful recovery action at such a low level, and how often will it just need to return some type of error value or raise some type of exception anyway, so that code further up the call stack with more information or resources available can deal with the problem effectively?
The idea of ensuring that you always have "meaty" functions is unhelpful, what's important is thinking about things from a perspective of abstraction, modularity, readability, and correctness.
OK, but readability and correctness are also global properties, not just local ones, and the benefits of modularity are closely related to how much complexity is encapsulated and hidden away by the extra abstraction, but one-liner functions rarely increase the level of abstraction very much.
Of course there are bad abstractions. A simple test is anything that doesn't immediately refine your code into something more closely resembling the language-spec. The goal should be code with just the right amount of abstractions such that they map to this language-spec with the least amount of friction possible.
DRY incurs a significant dependency cost. This has been painful for me, since I work on distributed systems a lot. When you have an ardent follower of "DRY" who refactors a common utility so that many subsystems of a distributed system have a code dependency on the same piece of code, then changing that piece of code can become very costly. When that cost gets too high (e.g. requires coordinated deployments and lots of cat herding with owners of other systems), then the DRY has just painted you into a corner where the cost of changing your code is higher than it's worth, and you end up with a rotting code base because the cost of change is too high.
Please, please factor that cost into your decision to wield DRY in distributed/service-oriented systems!
Code dependencies have diminishing returns in increasingly distributed systems...try to find a way to encode the common functionality as data instead of code and you'll be way better off.
So yes, you can just "rehydrate" the dependency into each component, but you often end up needing a fair bit of work for each component to untangle the mess and create the minimal dependency that each component actually needs. If you just want to copy the dependency directly, then sure, that's typically pretty easy. But that's also typically a pretty terrible outcome as now you have tons of duplicated code and modifying the dependency implies duplicate work, and worse, duplicate pointless work as you must maintain the pieces your component doesn't even use.
I think DRY is taken to extremes and you get that one-line function that does nothing meaningful except rename append or something equally trivial (rename append then append space).
I like small code bases. I like only have to make one bug fix. I like being able to rewrite large chunks of code (like entire protocols). And DRY lets that happen. But I also like readability, and something that can fight against DRY I realize.
I'm a pragmatist at heart. And coding is really a craft. So all these maxims have their use, but it just takes experience to know when to use and break them. But overall I see DRY as one of the more vital.
I like to think of this as: DRY (Don't Repeat Yourself) needs to be weighed against DRY (Don't Refactor... Yet).
This can be mitigated somewhat by liberal and thoughtful use of SemVer or equivalent.
(There is also, of course somewhat tongue in cheek; WET - Write Everything Twice)
Is this a piece of code that actually requires co-ordinating all nodes so they're running the same version of the code? Otherwise I'm not sure I see the problem, since you can just run old versions of the shared code in parallel with new versions.
Of course you can refactor to reduce dependency scope at a code level(the essence of functional programming), but if in order to do X you need to access W, Y, and Z, then I don't see how code changes will affect this deeper truth.
Repeat yourself. It's good for you, and makes you code faster.
You can refactor out later if code becomes bloated.
I'm not religious about DRY, but it's not a bad thing when kept in the toolbox with KISS and YAGNI.
If you can copy-paste repeated code, you can copy-paste debugging as well.
Duplicate bugs are a single bug.
Obviously not all DRY is bad.
So now not only have I created a bug, duplicated it N times, but my boss thinks I'm an idiot and wasting time for not applying DRY and good engineering hygiene.
Maintainable goes out the window...
The only thing I'd add is that you should think about including some comments when you duplicate code so that people realise how the different bits of code relate to each other. If you've thought of a potential way to abstract the code, but didn't proceed with it for some reason, you can add a note describing how it could be done. This helps new people understand why they're seeing duplication and how they can factor the code if they choose to.
On the contrary, you might want to factor out single tokens, if they are likely to change together.
You can view concerns over magic numbers as a special case of this. If you have "10" in 10 places, you probably want it in a constant with a name. But perhaps you want it in several constants with different names - does the "10" here represent the same piece of knowledge as the "10" there?
There's no reason the same doesn't apply if we want to perform the same operation in 10 places. Those that represent the same piece of knowledge ("this is how we render a text box", "we have capacity for 4 widgets", "we compute the total price of a basket like this") should likely be consolidated. Those that represent different pieces of knowledge should probably not.
This can be true for logic as well. If you have a very specific conditional it can help to put it into a function so that it's purpose is clear, especially so if it's a piece of business logic that might change. Interestingly this can be "anti-DRY" as well because you might have the same logic duplicated in different places, but the fact that they are the same is a coincidence, and you want them to change depending on their own requirements.
Which is funny, because as originally coined, it is exactly "DRY".
"Every piece of knowledge must have a single, unambiguous, authoritative representation within a system".
https://en.wikipedia.org/wiki/Rule_of_three_(computer_progra...
Premature abstraction is as bad as premature optimisation.
In fact their effect on the code base is rather similar: complexity and obscurity in the name of some theory about a future benefit.
The complaint about readability in TFA is inexcusable. If I do not have a system for visualizing (or otherwise comprehending) the complexity of my system, then there's not much hope. Dependency graphs and "Find references" in my IDE help. If the only tool in the toolbox is looking at the code that fits in a vim console, then I'm going to have problems writing large applications. I am not that kind of genius. I have met and worked with that kind of genius (i.e. holding all the code in their head) and it turned out that around 1998 codebases hit a size that even those geniuses hit a wall, with project-ending consequences.
In my experience the software at an organization is often a reflection of its culture. Thick with leaky abstractions, interfaces, facades, and indirection? Years of frustrated mediocre programmers, each slightly confused by the previous generations' shenanigans, led by a clueless management team that care more about deadlines than deliverables. There are signs that people are trying to do better but the code takes years to show improvement as it resists change at every corner. A crystalline entity designed and driven by data, never doing more work than is required? A team that has had little churn and is led by passionate engineers often more concerned with deliverables -- at least at first, because it's often fairly easy to make changes when your code-base adheres to a well-defined algebra, is thoroughly tested, and even has a well-formed specification of its critical components.
DRY is as good an idiom as "measure twice, cut once." The difficulty is that software is often more complex to understand than a cabinet. However I can't slight it for being a bad idiom because of the wide variability in skill for such a difficult task as programming. We need something to teach new programmers that at least get them thinking about the abstractions they wreak havoc with.
His premise - having a senior engineer spend an hour a day for the first month helping the new employee with explaining the existing abstractions being used, the underlying design of various systems, etc. - would still be only about 20 hours, which is still only 1% of the number of hours that employee will spend in their first year - about 2000 hours.
As a result, I believe that armed with that knowledge, the new employee is likely to be much more productive, failing which, at least cause less damage to the code base.
I would say that the first example you mention - leaky abstractions et. al. - are just as much (or maybe more) due to poor onboarding as they are due to the frustration of mediocre programmers. There is a lot to be said for good process, which software engineering as a discipline falls short of quite consistently.
[1] https://www.amazon.com/Effective-Engineer-Engineering-Dispro...
I would wager that 90% of good software (by a most liberal definition) is designed to be that way by the methods employed to construct it. It's the leaders and managers who set those processes and standards. If they are well versed in the state of the art and understand how to make it work for the business you can end up with crystalline entity software. Most of my job is presently "hacking the process" rather than the code itself so that the team can produce the best, desirable results.
I'll check out that book, thanks for the recommendation.
feel free to ignore it, mock it and toss it aside if it leads to bad abstractions, highly convoluted structures or write-only code.
anyone that has had to do maintenance or adding features to a large OO codebase from the early 2000s will have seen vastly massive class hierarchies where following the path of execution is a roller coaster ride through 10 files for what could have been a 30 line function.
these days grep/vim/sublime/emacs/and the rest of the gang all have power search and replace editing functions. sometimes it's better for the whole world to just copy-paste-alter your code.
Amen!
a roller coaster ride through 10 files for what could have been a 30 line function.
sometimes it's better for the whole world to just copy-paste-alter your code.
Then what happens when you have almost a hundred copy/pasted slightly rewritten 15-30 line variations on the same theme? How do you refactor then? (Yes, I have seen this is production systems, and yes, it was very critical code!) As you say, it comes down to cost/benefit.
Basically, you just have to keep potential refactoring/rewrite costs down so that you are never trapped. Caveat: You can seldom predict the risks as well as you think you can. What you can depend on, is being observant to historical patterns in your codebase. It's hard to predict the business needs and the architecture in the future. On the other hand, it's often quite easy to see the historical trends in the codebase.
Really, the analogy of the physical file room is a great one. (Or if you're not familiar, a library, a tool chest, or any kind of physical inventory system works too.) You can have a file clerk that seems like "super-filer" because he never "wastes" time by putting files back, but this never works out in the long term. The same goes for a filing staff that never reorganizes the file room. Also, one can generally see the long term disaster developing long before it results in the dramatic disaster. You can see which shelves are getting filled up, and which drawers are getting overstuffed, usually weeks or months ahead of time.
The only thing that can happen - you understand the core function those 30 variations solve and introduce a full parametrised solution to the problem, then replace all places with calls to it. Generally there would be some way to tell where all these copy-pastes are using something not much more complex than a regex. But even then, maybe just leaving it duplicated is better.
How do you know the code you're touching has other versions that are semantically the same and should also be altered?[1]
How do you avoid having to fix the same bug several times because you bug-fixed one place but not the others?
How do you avoid the technical debt that builds over time when instances of the pattern within the codebase are each at subtly different "versions" with similar, but not identical, semantics (even though identical would have worked fine?)
[1] Of course, DRY code has the inverse: How do you know if an existing function to do what you want to do already exists so you can avoid duplicating the extraction?
Where my opinion falls today is: A slightly more complex solution is often less risky and more maintainable than the straightforward duplication solution because at least the complex one looks complex to a would-be maintainer who will at least be aware of things up front, whereas duplicated code can have a bunch of hidden costs whenever it's touched that won't necessarily become apparent until later when the presence of that technical debt throws a monkey wrench into unrelated plans.
Meanwhile, there could easily be transformative code that just computes some stuff you often need. In those cases altering one variant need not effect the others.
Knowing where these things were wasn't a problem. They were all on the class side of certain classes. (Yes, this was Smalltalk, but this entire subsystem didn't have a single instance variable in it!) I was on a team of 10, with some very smart guys. We all wanted to "understand the core function" in this subsystem but what it really was, was an object system, where objects were expressed as consecutive entries in a series of arrays. Every method resembled some kind of complex merge with multiple arrays and multiple incrementing indexes and varying side effects embedded in nested conditional logic. Only one developer understood the underlying object model, and she wasn't apt to share. Rather, it was the source of her job security. (Most days, she spent in the cafe on the 1st floor, reading a book, until she got notifications, then had to "consult.") If you pointed out the "unusual" nature of an entire Smalltalk subsystem without a single instance variable in it, she started talking to you about her PhD in Math.
No, you aren't such and genius, and myself and my colleagues such dullards, that we only needed you to show up and point out a few simple truths.
So what do you do if that's not practical? See my other comment in this subtree where I give more background information. Sometimes the cost/benefit doesn't work out at that moment. (And believe me, we would've loved to refactor that whole thing!)
This type of thing is going to be terrible regardless of if it was in one place, if you can't test it.
We did such a good job that we never touched it again. My next company or a place I interviewed could just not understand why I couldn't answer questions about something so fundamental to our success. Certainly I must be overplaying my involvement in the project.
The joke I make is that we spend all of our time looking at the trivial code in our apps. The more important it is the less time we spend touching it, which is just so backward from other industries and I don't know how we fix it.
It's a much better rule as originally stated than as misapplied.
That's not to say that there won't be exceptions, even to the original rule.
The compiler is also quite helpful. Need to change a class? Just change it and recompile. All the places it complains about are exactly all the places that need to be changed, and a targeted search+replace will get them all easily.
1) There's a higher level abstraction in your code. Abstract at your own risk, unless you have at least 3-4 instances, or your architecture absolutely requires it.
2) There's wrappers around libraries and you only use it a certain way. I'm OK with this here. Sure, I could copy-paste the same exact parameters each time I use it in my code, but I will just write my own fiddleFoo() and use that every time.
3) There's when things MUST BE THE SAME. You should abide by DRY even if you only have 2 instances. E.g., we have an SOA and we DRY our routing. Routing MUST match or else things break.
In the large open source project I work on, we have a prolific contributor who is systematically going through the entire code base, "refactoring" some of it in way the impairs my ability to read, understand, and maintain the code (even code that I originally authored). Over time, we've added scores of new macros -- as result ordinary C knowledge is now insufficient to read and understand what the code is doing.
As the author suggests, the guiding principle should be factor only when it results in a net reduction of complexity. In some cases, the cost of adding new abstractions outweighs the gains from Don't Repeat Yourself.
There is some value in having some blocks of code that can be read in isolation, even if there some bit of repetition between blocks.
That is, yes, ideally you have a test catch something. Realistically, you don't have 100% test coverage.
Still, when I'm worried about other programmers breaking something, I write unit tests. Library code should have good tests that document the purpose of the shared function. And my general experience is that when I break some shared library function anyhow, I'll see other tests going red. Unit tests are my first line of defense for this sort of problem.
... it becomes clear that you're actually dealing with two separate "pieces of knowledge".
Parameterization is simply adding adjustable knobs. For example, a function can take as argument a gigantic list of flags, where each particular combination of flags makes the function do something slightly different.
Abstraction is actually separating concerns. It's dividing a program in logically independent parts, whose correctness can be established individually, without worrying about the others. Abstraction means that software components only interact with each other through explicitly designated interfaces, often enforced by the programming language itself.
Parameterizing is easy. Abstracting is difficult. Unfortunately, parameterization without abstraction is a source of headaches in the long run.
There are two kinds of generalizations. One is cheap
and the other is valuable. It is easy to generalize
by diluting a little idea with a big terminology. It
is much more difficult to prepare a refined and
condensed extract from several good ingredients.
DRYUYNTRYIWCRYBBCHYRY
The original formulation of DRY spoke in terms of information. Any piece of information should exist in exactly one place in your code. As I've said before, modulo some caveats related to double-entry bookkeeping, this seems exactly right. An inveighing against anything that looks repetitive is not.
The goal is not compression, it's ease of understanding and ease of making correct modifications.
I've started using "Huffman coding" to refer to the practice of misapplying DRY by deduplicating superficial similarity rather than information content.
There are times when repeating yourself is just accidental. I might have the same middle name as a coworker. Do we actually "share" that middle name? That depends on who's asking, or what system you're trying to build. But probably not.
Searching for the right abstractions is definitely worthwhile - even if it sometimes incurs extra rewrite/refactor churn. The better the abstractions used, the easier the refactoring, the faster you find the good abstractions, the easier it is to change the code as your understanding of the problem grows ...
I like this take on when go full DRY or not a lot. It's very easy to abstract out two things that look identical in their earliest implementation, but are actually different in intent and function--then when you get a new feature request on one of them they become obviously wildly different, and then to maintain the abstraction you have to throw in a bunch of conditional blocks into it and it turns into a nightmare to maintain.
I heard a rough rule of thumb to stay away from abstracting anything until you see it repeated identically at least three times, not two. It's just a rough rule of thumb to try and make sure you're not prematurely abstracting something and building the wrong thing, but anecdotally it's been useful for me.
I used to work in a shop with that rule, and I've mentioned it here on HN. It's still pretty easy to track down and rewrite 3 occurrences. Heck, it's still pretty easy to track down and rewrite 7, though as you let the numbers increase, you run the risk of missing an occurrence, making a mistake when you do the refactor, or the introduction of a confounding "idiosyncracy" in one of those occurrences.
In my experience, 2 occurrences is too little data to justify refactoring around, unless those are pretty hefty chunks of code. 2 occurrences of 2 consecutive lines is definitely too little.
I've not heard that before, but I like it. I have always advocated for waiting until there's at least two instances, with a similar reasoning: don't abstract until there's a 100% proven need to do so.
> It's very easy to abstract out two things that look identical in their earliest implementation, but are actually different in intent and function--then when you get a new feature request on one of them they become obviously wildly different
On top of waiting for the proven need, I think the other big thing you should strongly consider when building an abstraction is the future of that code.
If you need to make a change (regardless of what that change is), are you fairly certain every instance of it going to change in the same way? If the answer is not an immediate and resounding 'yes', then you should really stop and consider if your abstraction is actually a helpful step towards DRY, or merely clever "architectural astronautism" that is only going to be a pain to maintain later.
That should be all obvious to anyone with 10+ programming experience, imo, but I always see such "oh, dry or not, kiss or not" jitter. We now have great tools to do everything, except programming. Someone must make programming app better than just notepad-with-word-completion junk.
Replacing functions with some kind of copy paste tracking editor does not sound like an improvement.
I can't remember the name, but it will identify what it thinks is the original, and show where the copy/paste is, along with the edit it thinks was missing. This was usually around some error handling logic in our code base. Think:
if (checkIsValid(foo)) {
printf("Looks like foo is not valid. foo == %s\n", foo);
}
Long story short, advanced static analysis has come a long way.
I'm less concerned about duplication in code that uses data than I am the code that determines whether the data itself is correct.
http://harmonia.cs.berkeley.edu/papers/toomim-linked-editing...
In C++, on the other hand, the barrier to follow DRY is rather low as a I can literally move code to a helper function and easily parametrize it for reuse in another place.
On top of that, when 95% of the code you work with is based on a "Minimal Viable Product" (the bane of my existence), assumptions will probably be made incorrectly, or in a way that adds significant technical/organizational debt. By doing it yourself, the original assumptions can be put into a different light and can help reduce all sorts of debt in the future.
Yes, this 1000 times! DRY or any pattern/principle is not some sort of universal truth/declaration of ultimate goodness from your deity or some deep law like the Heisenburg Uncertainty Principle! It's just a rule of thumb. As a practical technologist, you must always implement with respect to the contextual cost/benefit. Taking any principle as an automatic ultimate good is simply intellectual laziness!
I was in a company with a Java product and a Smalltalk product. The Smalltalk product was rather well factored. We had a policy of rewrites to keep things architectually clean. We had the fantastic Smalltalk Refactoring Browser as our IDE, and we weren't afraid to use it. (To this day, you can refactor an order of magnitude or more faster with it than you can in other languages.) The Java product consisted of many independent parts with tons of duplicated code between them. Definitely not DRY. (There were also 20X more Java programmers than Smalltalk, for a comparable product.)
One thing I noticed, is that bugs on the Java side were more prevalent, but never affected the entire application. Whereas, the far better factored Smalltalk product could occasionally have the rug pulled out from underneath the entire application by the introduction of a library bug. (Though it would be very rapidly fixed.)
Another thing at that shop: We had a policy of only empirically verified DRY. We never wrote code to DRY in anticipation: only in response to actual code duplication. Generally, actual practice and experience is better at producing exactly the needed architecture than the prognosticating cleverness of programmers.
EDIT: Shower thought! (Yes, I literally got out of the shower just now.) There is a lot of emphatic but somewhat thoughtless application of principles and rules of thumb because it's actually motivated by signalling. The priority is actually on the opportunity to signal and not on the underlying cost/benefit calculation. There is a nice analogy for this in music. Once a musical scene gets a certain degree of prominence and/or commercial success, people start prioritizing sending the signal that they are part of that musical scene. So you will find bands and musicians using those techniques and stylistic flourishes, not because of the underlying aesthetic principles in support of the particular piece of music, but as an opportunity to signal. This happens in OO and also in the startup scene. (And, OMG, but is signalling given a lot of energy in the Bay Area!)
So the insider move is this: Look for the underlying motivation behind the signal. Did this person jump on the opportunity to signal, or did they first seek out the information to make the determination of cost/benefit? How carefully did they do that? (Hopefully, we can get some of the hipsters/pretenders to at least go through the motion of doing the above. Even just going through those motions turns out to be beneficial.)
And when those developers discover TDD, then their code spends multiple pages saying nothing at all. Oof.
I think deep down most of us intuit that this is a problem. We will persue an articulate but not particularly bright candidate and pass on someone who quickly arrives as the solution without showing any work.
But we let in people with good verbal and poor written skills and they create deserts, their code is so dry. No, I don't want you to explain to me again how great your code is. I just want to be able to follow it without using a debugger. Because maybe this bug isn't even in your code and I just want to make a determination and keep moving.
As someone more eloquent said, they are so afraid of making a bad decision that they try not to make any at all. Everything is named neutral words like 'context' or 'options', the verbs are neutral too, and there are other code flows that use the same name with different data.
As a general rule, if you would describe a code flow out loud in plain English, and not use any of the nouns or verbs that appear in the code, someone has ruined that piece of code.
No it is never a good thing because the code you write is the code that has to be tested and managed. If a routine you're going to copy paste 15 times has a bug you might not give a damn but the guy after you who has to clean up the mess might prefer this routine to be only in 1 place. The rule is simple, if some substantial logic is used at 2 different places it has to be refactored into its own component. Just like a variable has to be declared if a data is used at 2 different places, that's basic CS. If one has hard time to figure out what is what then it's a naming and documentation problem, not an "DRY code" problem.
The next day or later I come back to the code and sometimes I find a neat abstraction then, which just needed more context. Sometimes I can get rid of the whole section too...
It all... Depends.
Let the actual need for duplication guide you in finding abstractions, so you don't waste your time and produce difficult-to-follow code for no reason.
I tend to start structuring parts of applications and algorithms on paper, and this half hour with paper and pencil helps a lot.
I think "write the least code", from which DRY is derived from is universal.
It's just in competition with a few other axiomatic principles - typically the rule of least power and loose coupling, which can suffer when DRY is maximized at the cost of everything else.
>We had a policy of only empirically verified DRY. We never wrote code to DRY in anticipation
Can't agree enough with this one. The worst kind of re-factoring is that done in anticipation of future code.
These kind of trends in HN, that now I'm seeing more often, make me terribly sad, it's like promoting mediocrity instead of a well thought solution just because of laziness. If you can't understand some abstraction then try harder, or if the abstraction is not good then find a better solution. But PLEASE don't start to copy and paste and above all don't start promoting this abomination.
Source: I wrote a lot of copy and paste when I was too young to understand and I had to read too many copy and paste when I stopped doing it.
What happens when you are under deadline and you understand just enough to make a bad abstraction, but not enough to make a good abstraction? So here, you seem to have an underlying assumption of timeless omniscience. (This is common in people attracted to absolutes and abstractions.) By all means, if you have the time to make the right abstraction and DRY, then please do that. However, it's just as fallacious to suppose that any DRY abstraction is necessarily the right thing to do. Again, it's all about contextual cost/benefit. (Most of the time, it is the right thing to do, but this certainly isn't absolute.)
To be more specific, what if you have an abstraction that lets you do DRY, but it's obtuse, it looks like it's not quite the right answer, and it would involve the modification of a library that is used by many other parts of the system for which you don't have tests? Are you certain that in all cases like this, the cost/benefit of not having to do the search in your code next week is going to be worth it?
I find that many so called best practices propagate because of how engineers want to be perceived. You want to be seen as the smart guy who reads the Gang of Four book so you shove OO patterns where they don't belong. Or you let others blindly follow dogma because you don't want to be seen as the dummy who just doesn't get it.
Seriously, a comment that goes like this:
"2009 sept 20 : Peter Pan: this abstraction is to hid the ugliness of using SQL Server 2003 which is still in use by Slow Corp."
... now in 2016 a maintainer knows the assumptions and the reasons for the abstractions and can make a much clearer decision about how the abstraction should be treated.
Any time you call a function twice with the same arguments, you should religiously write a two-iteration loop around just one call.
Any time you have more than one two-iteration loop in the same program, you should write a 'dotwice' macro and use that instead.
Any time you have a function call sitting in a loop just for the sake of being repeated, you should avoid the argument expressions being evaluated multiple times; you must religiously evaluate the arguments to temporary variables outside of the loop, and in the looped function call, refer only to the temporaries.
In order not to repeat this pattern itself you need a proper "calltwice" macro, and to be working in Lisp, ideally: (calltwice yourfunc expr ...). yourfunc and each expr are evaluated once, then the resulting values are applied to the function twice.
This is a DRY town, damn it!
In fact, I think this talk is superior to the linked article. You should consider submitting this talk directly to HN.
While we're at it, why don't "we" just "ban" - mathematical proofs without numbers - all criticism, as it's about art but contains no art - all papers in the field of music theory, since they aren't music.
I don't think those are good comparisons. Better example would be a post about how to write rigorous math proofs and then not giving a rigorous proof as an example.
Oh, you mean like some of Dijkstra's numbered papers? https://www.cs.utexas.edu/users/EWD/transcriptions/EWD10xx/E...
I think "examples might help" is valid feedback, but the author has sort of provided a reason why examples aren't included: in small projects the abstractions are usually worth it, was my read. So while the author could talk about examples, he probably couldn't include then in a piece of this size.
"Can we ban" is what has rubbed me wrong way. Who is "we". How are "we" "banning"? Does this just mean you don't agree with this post being highly upvoted? Or that you want to circumvent the opinions of those who upvoted with a "ban"?
It is interesting also to think about the irony of an argument that one should work in terms of concrete examples of the subject matter, in objection to a piece that argues one should not always abstract things.
Another big issue in the framework case is the inversion of control, which further restricts how the reuse can happen and makes it very hard to reuse the code differently in an efficient way.
However, language features are not the only abstractions you have available. In fact, they're typically the least interesting ones you could apply to a problem - any yokel can lift code out and slap parameters, annotations, reflection, or generic type signatures on it. But to reach interesting abstractions, you have to be patient enough to do discover the shape and flow of the code over time and subsequently apply exactly the right data structures and algorithms to either reproduce the same shapes in an abridged form, or different shapes that achieve the goal more efficiently.
So when I position myself as anti-DRY, which I often do now, I'm saying, don't succumb to the thinking that just shovelling things around and slightly repackaging them will add up to what you wanted. An actually useful abstraction cuts so deep that it may amount to a different thing altogether. The language features are there to solve very straightforward situations that are known to reoccur time and again. Do not use them in clever ways.
I don't think this is as "extreme" as some perceive it.
Besides, research has shown that plain ole black on white is the best combo for readability.
An HN comment usually gets surprisingly better if you take the name-calling bits out of it. I'd say that's the case here.