19 ms·

I’ve usually heard this phenomenon called “incidental duplication,” and it’s something I find myself teaching junior engineers about quite often. There are a l

I’ve usually heard this phenomenon called “incidental duplication,” and it’s something I find myself teaching junior engineers about quite often. There are a lot of situations where 3-5 lines of many methods follow basically the same pattern, and it can be aggravating to look at. “Don’t repeat yourself!” Right? So you try to extract that boilerplate into a method, and it’s fine until the very next change. Then you need to start passing options and configuration into your helper method... and before long your helper method is extremely difficult to reason about, because it’s actually handling a dozen cases that are superficially similar but full of important differences in the details. I encourage my devs to follow a rule of thumb: don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. Let the code go through a few evolutions and waves of change. Then one of two things are likely to happen: (1) you find that the code doesn’t look so repetitive anymore, or, (2) you hit a bug where you needed to make the same change to the boilerplate in six places and you missed one. In scenario 1, you can sigh and say “yeah it turned out to be incidental duplication, it’s not bothering me anymore.” In scenario 2, it’s probably time for a careful refactoring to pull out the bits that have proven to be identical (and, importantly, must be identical across all of the instances of the code).

teh_klev 7y ago
> don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. Let the code go through a few evolutions and waves of change. This ^^^. I'm in my mid 50's now and worked as a software dev since my teens. I've learned over time that certain lumps of code need to be left alone for a while before jumping in and aggressively refactoring the perceived "duplication". I've been guilty of this kinda thing before, spot a wodge of code that looks like a duplication, only to find out later you hit some "special cases" as the project progresses and suddenly, as the article points out, your "helper" method balloons into a ball of spaghetti. As an apropos to the article, and touched upon therein, checking in a fairly major change to de-duplicate some code without consulting the original author/team is a wee bit rude. Ask your colleagues first why such code still exists before barging in and making these changes, they may already have some concerns as to why refactoring to a helper method or some abstraction isn't in their game plan yet. It's a common courtesy.
- kqr 7y ago
  > As an apropos to the article, and touched upon therein, checking in a fairly major change to de-duplicate some code without consulting the original author/team is a wee bit rude. This sounds like the outcome of bad culture. Ownership of the code should be shared to the point where it should never be considered rude to improve the code. Any part of the code. > Ask your colleagues first why such code still exists before barging in and making these changes, If I had to synchronise with others all of my improvements to existing code (which was frequently written in a hurry to meet a deadline, so with shortcuts taken intentionally, or with incomplete knowledge of future use cases) I would get at most half as much done. > they may already have some concerns as to why refactoring to a helper method or some abstraction isn't in their game plan yet. If there are alluring "improvements" that don't work for such subtle reasons, this should be documented in the code. If it's not, one has only oneself to blame when someone goes in and changes it. Edit: I realise now that I'm talking about teams where everyone is reasonably senior. It could be different with junior members on the team, to which many changes might look like improvements when a senior engineer would at most see the change as equivalent. In that case I think you're right, but for a different reason: junior engineers should always check in with senior engineers about things in order to learn more of the craft!
  - _pmf_ 7y ago
    > This sounds like the outcome of bad culture. For 90% of shops outside the cornucopia of SV with unlimited budgets and internal customers only, the client drives decision mercilessly and ruthlessly.
    - dx034 7y ago
      And for a good reason. The product doesn't exist because they're fun to develop. They exist because they solve a problem customers have. So ultimately, decisions should always be based on what customers (long-term) benefit most from.
- mytailorisrich 7y ago
  This isn't about rudeness or courtesy unless they are responsible for that code, in which case they should be involved or leading the change. As you mention it's about technical knowledge: They know exactly why the code is the way it is so consulting them is "due diligence".
Mirioron 7y ago
I wonder if we could have IDE features that will make #2 less likely. Eg marking some lines of code that they're similar to lines of code elsewhere. And if you then change it in one place the IDE will remind you about the others.
- lonelappde 7y ago
  Relying on your IDE to maintain program semantics is terrible. IDE should help you write good code that you commit, not be a crutch for writing bad code.
  - Mirioron 7y ago
    People in the thread are arguing that it's not necessarily bad code though. Also, the point of the IDE would be to help you just not forget to change it.
- hising 7y ago
  JetBrains has something like this in their tools. It pointed me in the right direction a couple of times when touching code that is similar to something elsewhere. https://www.jetbrains.com/help/idea/analyzing-duplicates.html https://www.jetbrains.com/help/idea/analyzing-duplicates.htm...
  - hising 7y ago
    Misunderstood the question (I just realized now ...). However. The tool in JetBrains is good at finding duplicates ...
JamesBarney 7y ago
I ask myself a couple of questions to decide when to abstract code to de duplicate it. 1. Is this domain logic or plumbing code?(for instance would this logic be described in a spec for the application) 2. How many times will these be duplicated? 3. How abstractable is it? 3.a. to abstract it will I have to do anything tricky like pass functions around use reflection? 3.b If I abstract this logic into a function is there a good concise name for it?
detaro 7y ago
And in the other direction, when adding flags etc to a method that was created to reduce duplication, consider if splitting it into two "duplicate" copies or copy it to the call-site requiring the flag isn't the better alternative.
- danabramov 7y ago
  Yes. If extracting and deduplicating is in your toolbox, so should be inlining and duplicating.
- ahartmetz 7y ago
  Or, if possible, split up the helper into smaller units of functionality that can be combined as appropriate for different requirements. That could possibly work for the original article's problem, too.
scarface74 7y ago
That can be boiled down to the “Rule of 3”. My CTO often asks me to implement a feature to do X and make it “generic enough to handle future use cases”. My answer is always the same - either give me at least three use cases now or I am going to make it work with this one use case. If we have another client that needs the feature in the future then we will revisit it. Of course, there are some features that we know in advance based on the industry how we can genericize it.
- deleted 7y ago
  [deleted]
- rdiddly 7y ago
  "Premature optimization something something..." ;)
  - lolc 7y ago
    That's actually "premature generalization".
    - sverhagen 7y ago
      Isn't premature optimization a generalization of premature generalization? ;-)
- RealDinosaur 7y ago
  I think the catch all term for that is YAGNI.
  - pytester 7y ago
    YAGNI is about not adding functionality until it's needed. DRYing code isn't adding functionality.
    - philipov 7y ago
      I think GP is implying the thing you're not going to need is a generic solution to the repetition. i.e: DRY is in fact a feature.
yodsanklai 7y ago
> Then you need to start passing options and configuration into your helper method... and before long your helper method is extremely difficult to reason about In which case, you should split the helper function ( extract sub-part common to all cases, and report the differences where the helper function is called). I think I would most of the time go with de-duplicating as early as possible, as long as the helper function only has few parameters and can be described in plain english rather easily. To me the cost of having to refactor the helper function later in the process is less than dealing with duplicated code. Duplicated code causes many issues. You mentioned introducing bugs, but it also makes the code harder to read. Every person who reads the code has to make the de-duplication effort mentally (check that the duplicated parts are indeed duplicated, figure out what they do, and on what parameters they differ...).
- zmmmmm 7y ago
  > Duplicated code causes many issues. You mentioned introducing bugs, but it also makes the code harder to read That is I believe contestable. Yes, it can end up easier to read but there is a big tradeoff - when you remove code from its context it is much harder for a reader to reason about it. Once something is extracted into a function you can't see, you have to mentally ask the questions like "could this return null / None?", "how will it behave if the input is negative?", "does it throw exceptions", "is it thread safe?" etc etc. All this is directly observable in context when the code is inline, not so when it is removed to a central place.
  - yodsanklai 7y ago
    Ideally, these questions should be answered by the function name, type, and its documentation. And if not, one can always jump to the function definition and the above elements (missing if the piece of code is inlined) will give additional hints to what the code does.
  - deleted 7y ago
    [deleted]
- barrkel 7y ago
  Premature optimization. Duplicated code is only evil when there's a bug you only fix in one place and forget about the duplicates; in almost every other case, it's easier to reason about and is more resilient in the face of local changes. Abstraction is often like compression, and compressed data is easier to corrupt. Change the implementation of an abstraction, and you put all consumers of it at risk. It's not an absolute good.
mfateev 7y ago
Go error handling is a good example of this. So far all the attempts to reduce the repetitive `if err != nil { ... }` through some abstraction failed. Look at https://github.com/golang/go/issues/32825 https://github.com/golang/go/issues/32825
- asdfasgasdgasdg 7y ago
  I don't really think Go error handling is a good example. A nil test paired with a return cannot be extracted to a function. A macro could perhaps centralize the logic, but otherwise it's no more amenable to deduplication than plain addition. Moreover, it's completely trivial, if verbose in the Go language. The case where it becomes interesting is when there are four or five statements that are repeated. If it's two statements, especially if the statements are both control flow, one of which is tied to the frame of the executing function, and the other is a trivial nil test, that falls firmly on the "not refactorable" side of the line.
- renox 7y ago
  In the language maybe but both Rust and Zig show that it's possible to have much less 'bloat' for error handling even without using exceptions. I'd say that go designers have still work to do: Zig especially show that you can be a 'simple' language and yet have both sane error handling and generics.
  - arendtio 7y ago
    If someone else is looking for examples too, I found those: https://www.reddit.com/r/Zig/comments/99zlc9/exceptions_or_errors/ https://www.reddit.com/r/Zig/comments/99zlc9/exceptions_or_e...
    - DelightOne 7y ago
      On the error branch, how do you do code coverage?
    - marcus_holmes 7y ago
      I thought all of this until I got used to Go's error handling. There's a couple aspects to this: 1. After a while, the "if err != nil {" becomes a single statement in your mind, and you only notice it if it's different (like trapping things that should error with "if err == nil {"). In other words, it only feels verbose if you're not used to it. After a while, the regular rhythm of "statement, error check, statement, error check" becomes the routine pattern of your code and it looks weird if you don't check for errors (which is as it should be). 2. The point of Go's error handling is that it isn't magic. There's nothing special about error values, and they are handled exactly the same way as every other variable in the system. The only thing the language defines about errors is that they have a method called Error that returns a string. That's it. This means that you can create complex error handlers if you need it, entirely within the standard language. This is extremely powerful. The Go team's examination of the language error handling is interesting because it seems there's a conflict between newer Gophers who don't like the verbosity of it (but don't realise the power it brings) and the older Gophers who are used to the verbosity and appreciate the power. Almost exactly like TFA. The repitition looks ugly if you don't appreciate the reasons for it.
m0zg 7y ago
And a rule for Java programmers as well: nobody is going to "extend" your program. If your interface has only one implementation, you do not need the interface at that time, and possibly ever. Nor do you need DI or whatever other masturbation "best practices" Java gurus prescribe. When it comes down to it, Java is a simple, pleasant language. So people invent all sorts of indirection to appear smart. Don't. Just do it simply and readably, with as little indirection and abstraction as possible.
konschubert 7y ago
you can also try to reason about the situation to figure out if the duplication is incidental or inherent.
lilyball 7y ago
I have 2 rules I use when determining whether to duplicate code or to refactor: 1. How many duplications are there? If the code is duplicated once, that's fine. If it's duplicated twice (so 3 instances of it), then it's time to consider refactoring, subject to the next rule. 2. Why is the code duplicated? If it's "incidental duplication", i.e. code that happens to look the same, don't refactor. Only refactor if there's actually a reason why the code is the same. Which is to say, attempt to predict the future: if a change is made to one instance of the code, do I expect it to be replicated to the other instances too?
- Hendrikto 7y ago
  > attempt to predict the future Sounds very error prone…
  - marcus_holmes 7y ago
    This. I wait until the feature is mature, and has been used by actual people. There's no point trying to clean up code that hasn't finished evolving, or that I don't understand fully. It also means I have a tidy stack of refactoring to do when I'm bored or need a quick motivational win :)
  - lilyball 7y ago
    It is, which is why you want to be conservative. If the duplicated code is obviously supposed to be identical, then predicting the future should be trivial. If it's not obvious, then it's a question of "can I conceive of a reason why I'd want to update one and not the others?". And if the answer is unclear, wait a while and see if anything comes up.
- jrumbut 7y ago
  It's also useful to look at not just the duplication but the code itself. In this case, it was code for geometry which is not like to change all too much. Often the difference between harmless but ugly looking duplication and duplication that is actually harmful relies on the semantics of the code and not just its appearance.
daxfohl 7y ago
I call these "spaghetti abstractions", and they're the worst kind of spaghetti.
mhh__ 7y ago
Good generics can be really helpful here. Can't be specific without knowing the exact helper function but I find sitting down with a cuppa (away from the computer) and planning an interface (If it's C++) that accepts (say) a policy class with a default and a callable object ("Functor") very helpful. Sometimes it can't be done but it's better to have a bugfree building block (Macros don't count!) that can accept buggy user defined tasks than lots of repitition.
- morelisp 7y ago
  > Macros don't count! Well, C++ macros don't count.
skipants 7y ago
Agreed. And more importantly, (2) is waaaaaay easier to fix than the problem you outlined in your preamble.
babarock 7y ago
One of the areas where I really like "incidental duplication" is in tests. Tests can sometimes be very repetitive and identical, and it's tempting to want to refactor it in some clever way. That's almost never good. On top of the reasons laid out in parent comment, tests also function as unofficial documentation. I like having everything explicit in there, it makes them easier to read and understand.
- lonelappde 7y ago
  Terrible acronym for a good idea: DAMP stackoverflow.com/questions/6453235/what-does-damp-not-dry-mean-when-talking-about-unit-tests
- blazespin 7y ago
  Tests rarely have bugs, I find, so generally dry isn’t critical. Also, dry is for security (see below)
  - lonelappde 7y ago
    Tests have fewer bugs if you write them before the system under test, and if they don't have mocks, and if you have enough of them that anything you get wrong the first time will get noticed by the results of the many similar tests.
    - gusmd 7y ago
      > Tests have fewer bugs ... if they don't have mocks 100x this. I've repeatedly fail to convince my team members that mocks are unnecessary in most cases. I've reviewed code with mocks for classes like BigDecimals and built-in arrays. This is especially prevalent in Java teams/codebases. Drives me insane.
  - smnplk 7y ago
    As a preventative measure, I write some tests for my tests. Also in TDD style of course. And on a very rare occasion, I have to write a test for those tests as well.
BubRoss 7y ago
One big way I prevent this from happening is to treat classes as interfaces to data structures and keep everything that isn't about accessing the data elsewhere. Conversions to other data types go somewhere else. In fact I don't want my data types depending on any other data types at all. When doing this any of this repetition or evolution can stay out of the data structures themselves so that they can be reused without irrelevant baggage.
- StevenWaterman 7y ago
  Have you not simply abandoned OOP at that point? A core point of OOP is that objects manage their own state, and provide an interface for accessing/mutating it. If classes are only used as data structures, and everything is done through (presumably pure) utility methods, it sounds like you're writing procedural code in an OOP language. That's not inherently a bad thing, but OOP provides benefits and you may be making a trade-off without thinking about it.
  - BubRoss 7y ago
    I try to worry much more about what works rather than labels. If you stuff all your functionality and data transformations into class definitions, your class definitions will be full of dependencies. Now all the fundamental elements of your program depend on each other and can't be separated. It's like trying to pile up concrete until it forms a cave instead of laying it down to make the floor and building on top of it. What am I missing?
c3534l 7y ago
Refactoring duplicated code is a trade off: on the one hand you create abstraction and centralizing code logic at the cost of, first, the overhead of learning that abstraction, and second, by increasing coupling between functions. I've personally found that the coupling is by far the most important factor to consider. If A depends on B, and C is found in both A and B, then you should factor out C. If A and B share C because they are adjacent, without being fundamentally intertwined, then duplicate C, but consider creating a library or module that makes it easy to talk about things that are similar to that duplicated code, C. (I tend to think about modules/libraries as little, independent DSLs)
soulofmischief 7y ago
Another thing you can do when a function becomes overencumbered is to split the remaining similar logic into smaller functions which are composed into specialized functions. This has benefit in that when analyzing modules, you can spot differences in procedure at a glance instead of needing to dig through 100 lines of somewhat similar imperative code.
- loopz 7y ago
  Right. There are two kinds of such refactoring / DRY-ing up the code: 1) Specialized helper sub-functions/classes to make the codeblocks DRY. 2) Functions/classes to make separate features DRY. Problem arises when the design or understanding of the code doesn't reflect the realities of changes over time, forcing you to restart/revert, or making spaghetti code with optionals and whatnot to accomodate the rising complexity. The agile approach would be to make the code that is easiest to change either way, and prevent being locked in to only one approach.
cheshire_cat 7y ago
Sandi Metz wrote a blog post about this: https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstracti... > The moral of this story? Don't get trapped by the sunk cost fallacy. If you find yourself passing parameters and adding conditional paths through shared code, the abstraction is incorrect. It may have been right to begin with, but that day has passed. Once an abstraction is proved wrong the best strategy is to re-introduce duplication and let it show you what's right. Although it occasionally makes sense to accumulate a few conditionals to gain insight into what's going on, you'll suffer less pain if you abandon the wrong abstraction sooner rather than later.
- BalinKing 7y ago
  Sandi Metz's blog (and book) are an absolute gold mine. I'm a junior developer (<5 years), and the material gave me an entirely new outlook on design (object-oriented and otherwise)... and also showed me how un-maintainably most development is done nowadays :( If there was a required reading list for professional developers, I would put her work on with zero hesitation, I feel it to be that important.
  - bberrry 7y ago
    Which book are you referring to?
    - steveklabnik 7y ago
      Not your parent, but https://www.poodr.com/ https://www.poodr.com/
      - xenihn 7y ago
        She has a new book too. https://www.sandimetz.com/99bottles https://www.sandimetz.com/99bottles
  - klyonrad 7y ago
    You call devs Junior until they have over 5 years experience? Wow, that’s harsh
SlowRobotAhead 7y ago
> I encourage my devs to follow a rule of thumb: don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. Let the code go through a few evolutions and waves of change. ... alright buddy, I’m going to need you to remove your spyware on my computer! I just did this last week with a CBOR message interpreter in C. I did it the long messy way, saw a bunch of repetition, “cleaned” it, changed the way the parser is called, changed some of the validator that runs after it... then did that entire process two more times as requirements changed. I’m near certain you wrote that ABOUT me! ;)
jdmg94 7y ago
inversion of control
wutbrodo 7y ago
My first job at a bigco had a very heavy read-the-code culture, and being able to read code and write readable code is one of the most valuable skills I learned there. The lack of this skill is one of the things I've found most frustrating about working with less talented or, now that I'm a bit later in my career, more junior engineers. There's a tendency to glom onto black-and-white, superficial rules without understanding them, instead of appreciating the principles underlying them and having a sense of how to balance trade-offs. This creates an unfortunate cycle: everyone writes unreadable code, so nobody practices reading code, so nobody internalizes what readable code looks like and continue to write bad code. I tend to have a strong reaction to duplicated code, but DRY is risky if whatever you're pulling out isn't logically concise and coherent[1]. Some of the helper functions I've seen in code reviews (as well as the one in the OP) strike me as written by unsophisticated language AIs, catching textual similarities that aren't semantically linked and pulling them out. The engineers I've mentored over the years, including ones starting with no eng experience, go on to write fantastic code (and no, this isn't just my opinion). But it's a very labor-intensive, hands-on process of thorough reviews and feedback until they internalize the underlying principles, and it can be tough to swing in very short-term-oriented company environments. Now that I'm running larger teams, I've been noodling over how to encapsulate this deeper understanding of what makes good code in a way that scales better. But the fact that Google et al haven't already done this makes me think it's not something you can teach with eg a bullet point list. > I encourage my devs to follow a rule of thumb: don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. Let the code go through a few evolutions and waves of change. (Note: this isn't a case of what I describe in the earlier part of my comment, as I don't think this is a superficial, black-and-white rule) I disagree pretty strongly here, especially since you're #2 involves waiting til you hit a bug. IME, there are many cases in which a solid understanding of the code allows pulling repeated code out in a way that's principled enough that it's more likely to adapt well to future changes. Indeed, this is true of most changelists to some degree, or you'd end up with no functions or classes other than main(). [1] A good rule of thumb is whether the free function has a reasonably readable name that captures its logic, without having to abuse handleFoo() or processBar().
- useragent86 7y ago
slim 7y ago
the rule is "do the simplest thing that could possibly work"
hyperpallium 7y ago
Code duplication is a suggestion of meaning identity. But can be coincidential. Consider LZ78 (represents duplication with backreferences). Not very human readable.
ianamartin 7y ago
I think that people often make these choices based on latent social and work-related contextual factors as much as there is any misunderstanding or blind adherence to the wrong set of guidelines. There are human factors as well that go into people defensively citing guidelines for their decisions after a criticism comes up in code review. It's somewhat likely that the choice of a premature deduplication wasn't deeply considered. The programmer just did it because they felt like it made sense, and when questioned about it, they claim whatever "rule" exists to justify it. There's a strong pressure in work environments to get shit done, get it through review with as little fuss as possible, and move on to the next thing. That's work after all. Choosing to leave something in a state that's guaranteed to need attention later can be viewed as lazy. If you at least make an attempt to do something in a clean way, it will be perceived as "work" even if it turns out to be the non-optimal choice. I find for myself, at least, that I'm far more willing to let things evolve in my personal projects and to discover the right abstractions over time than I am in my work projects. I don't know if that's true for everyone though. My personal projects are always about the learning process. Maybe for a lot of people that's not the case? The focus is more on the product than the process? I write the first version of every side project in a language I don't know or don't know well and rewrite the actual version I'm going to use in a language I do know well. So I'm really happy to let things evolve, and the more I know the domain in one of my main languages, the more time I spend in the new language because I want to distance myself from what I think I know and examine it fresh when I come back to it and see if what I think I know is actually still (or was ever) valid. Different people have different teams and motivations both at work and at home, so while I agree with you that taking a wait-and-see approach is often a really good idea, there are often human factors involved in these decisions where the same person will make very different choices depending on their environments and motivations.
vbezhenar 7y ago
Another approach with statically compiled language and good IDE might be to reduce duplication whenever you see it and inline back when you feel that this abstraction is no longer useful.
watwut 7y ago
Extracting common function was right move in your example. The bad move was to add options to common functio instead of changing one caller to call new different method. Even worst move was add more and more options to originally simple method. Just because something was rightfully extracted to common place 10 months ago dies not mean it have to remain in common place. And the need for split does not mean original extraction was wrong.
vonseel 7y ago
I’m just wondering where you draw the line. As with everything, it often becomes a big grey area on what is acceptable and what is not. Example: my (fictional) company sells a B2B platform which provides companies with an online marketplace or some other type of online application. Each installation requires different, though often similar, integrations with the customer back-ends - think Postgres vs MySQL, but some customers may also use TotallyEffingObscureDB, which doesn’t even speak SQL. Those backends are usually the source of truth for the entire stack, storing user data, etc, the local data is just a mirror. So, given this scenario, how should we approach user registration processes? Is that a product component or custom (not DRY) code? What about all the other (similar, common, and basic) features of our platform that every customer wants but invariably needs to be implemented slightly differently? I’m only posing these questions because I worked in a situation where something similar happened, and it wasn’t dealt with well, and I’m wondering what other HN coders have to say...
- Fanmade 7y ago
  Ironically, we're handling this with the concept called "clean code". We do have a core, which does implement the base logic. Everything in there is domain driven, but only using DTOs and providing interfaces for input and output, using repositories and presenters. When the data source changes, we only need to add the new repositories and set those within the context. If we have to implement some specialized logic just for one client, we add this as a plug-in on top, so it is handled outside of the core logic. Of course all of this is very abstract and since I've worked mostly with simple MVC concepts until now, I'm still struggling to get my head around this approach, but so far it's looking very well. It may look overcomplicated at first, but there is a very strict while at the same time very flexible logic behind it, which can handle all the "creative" ideas our clients have so far, while still keep being maintainable. It is a long process to get to this method of programming, but my initial scepticism has completely changed to "why haven't I worked like this before?".
  - qlk1123 7y ago
    Interesting. I'm curious how long did your core system with basic-logic take to reach that maturity, and how many people involved? What kind of development model did you use? Also you state that you "still struggling to get my head around this approach," does it mean that the system somehow violates the principle of least astonishment? (https://en.wikipedia.org/wiki/Principle_of_least_astonishment https://en.wikipedia.org/wiki/Principle_of_least_astonishmen... )
speedplane 7y ago
> don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. I have a similar rule of thumb: when you do a thing the first time, just get it working. When you do a similar thing a second time, just get it working. When you do a thing a third time, pull it together as a cohesive abstraction. This is a silly generalization, but the point is that you generally need to see a handful of examples of a thing before you can make any useful abstraction. What's more common is people creating abstractions during the first iteration, leading to features that are never used, and others that were never considered.
- sitkack 7y ago
  And the abstractions are not abstracting over the correct things so they are actually a net negative. One has to reason about the abstraction AND the problem domain. I find it helpful to work with ADTs of values, lists and maps. No OO, just functions for selection and projection. The majority of programming is figuring out the nuances of the domain and getting something working. Code is actually an impediment to that.
b0rsuk 7y ago
From my experience, I would start with a more compact implementation first in hopes that it would save me time. This usually ends with me having to do more complicated operations in my head to plan the code. It's tempting. It's like trying to do math in your head to save time - often leads to a mistake. Now I believe there's no shame in trying out a solution first and rewriting it later when I get a feel what the problem is really about. John Carmack once said something like "If you're not sure which version is better, code it both ways and compare". The only downside of later optimization is that in commercial environment they often don't let you clean up.
- _pmf_ 7y ago
  > The only downside of later optimization is that in commercial environment they often don't let you clean up. Exactly (speaking for custom industrial automation projects). The follow up next requirements for a project might be in 5 or 7 years or never. You would be cleaning up a 80% dead project. In these environments, it's not uncommon to no longer have a dev environment because the system is a singular testbench that costs upwards of 2 million and only exists in 2 factories, so all bugfixes must be extremely conservative and any refactoring is stricly forbidden because there's no way to locally test for regressions.
gvjddbnvdrbv 7y ago
Reusing a function like this is not clean code. It violates several principals. 1. Functions should have as few parameters as possible and almost never have flag parameters. This is a basic thing and costs very little to follow. As soon as you want to add a flag to a function you need to make a new function. 2. Minimize coupling. 3. Single responsibility principal. A unit of code should have one reason to change. Of course in order to follow principles 2 and 3 here you may well need to consider the business logic.
partyboat1586 7y ago
I think about it in terms of bugs. If your abstraction causes a bug, I have to go in and work out wtf your wonky abstraction is doing and also risk breaking other cases. If there are 6 duplications and there is a bug because one of them is missing a change applied to all of them, that takes 5 mins to fix and risks breaking nothing. When you make an abstraction think not only "Will this create bugs?" But also "If this abstraction does create bugs will they be easy to identify and fix?".
truculent 7y ago
The best way I’ve had this explained is that refactoring != DRY. Some methods have a similar structure, but if the problem they’re solving isn’t fundamentally similar then it’s not truly refactoring and you’re setting yourself up for more work in the future.
jkoudys 7y ago
It's important for developers to understand that programming is at least as much about expressing yourself clearly with language, as it is about maths and compsci views of functions. Language that's more verbose, but also adds clarity, is a good thing. If I had an instruction book on building a cabinet, it wouldn't help to re-list every screw, tool, and their sizes on every single step if I could put a parts list at the front. But it also wouldn't help to collapse every matching group of steps with one or two different parameters together.
kostarelo 7y ago
Loved the "incidental duplication" term. I will def start using it from now on. But to add to this, I want to say that the moment you find your self writing a "helper" function, that's the moment when you must realize that you didn't understand the problem quite good. As a rule of thumb and what I am always trying to pass to my peers at work etc, is to always try and think in terms of your domain and the models in it. - What models does this helper function is acting upon or on behalf? - Does it make more sense to create a model and put that function in it? - Should that function be in a specific model or more that one?
kazinator 7y ago
Not only DRY but YAGNI. Unless you are sure you will need to separately evolve the initially duplicated cases, assume YAGNI: You're Not Gonna Need It (the repetitions). Avoid repetition like the plague; when the new requirements arise that conflict with the reduced repetition, confront it at that time. It's possible they they are bad requirements, in which case the clean, DRY code can support arguments against the requirements. Treat the computer as more of an Analytical Engine, and less of a Jacquard Loom. The special case behaviors for various shapes hinted at in the article: > For example, we later needed many special cases and behaviors for different handles on different shapes. This sounds like it might be a bad experience for the end user who has to learn annoying shape-specific handle quirks.
Shinchy 7y ago
I really like this way of looking at it.
avgDev 7y ago
I am working on a large project and I also realized this. I worked really hard to not repeat myself. I tried to reuse as much as possible as not to create extra code. Then, I quickly realized the things are NOT EXACTLY the same, and the methods to handle things were similar they were not the same. So, I had to refactor a lot of other helper methods. I agree with the approach you are describing. It would be easier to refactor later, as one would probably have a better understanding of the application in the wild.
collyw 7y ago
I am coming to the conclusion that code reuse is over pushed as an ideal in universities. Often the desire to create reusable code means that we create overly complicated code with less duplication.

teh_klev 7y ago

> don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. Let the code go through a few evolutions and waves of change. This ^^^. I'm in my mid 50's now and worked as a software dev since my teens. I've learned over time that certain lumps of code need to be left alone for a while before jumping in and aggressively refactoring the perceived "duplication". I've been guilty of this kinda thing before, spot a wodge of code that looks like a duplication, only to find out later you hit some "special cases" as the project progresses and suddenly, as the article points out, your "helper" method balloons into a ball of spaghetti. As an apropos to the article, and touched upon therein, checking in a fairly major change to de-duplicate some code without consulting the original author/team is a wee bit rude. Ask your colleagues first why such code still exists before barging in and making these changes, they may already have some concerns as to why refactoring to a helper method or some abstraction isn't in their game plan yet. It's a common courtesy.

kqr 7y ago
> As an apropos to the article, and touched upon therein, checking in a fairly major change to de-duplicate some code without consulting the original author/team is a wee bit rude. This sounds like the outcome of bad culture. Ownership of the code should be shared to the point where it should never be considered rude to improve the code. Any part of the code. > Ask your colleagues first why such code still exists before barging in and making these changes, If I had to synchronise with others all of my improvements to existing code (which was frequently written in a hurry to meet a deadline, so with shortcuts taken intentionally, or with incomplete knowledge of future use cases) I would get at most half as much done. > they may already have some concerns as to why refactoring to a helper method or some abstraction isn't in their game plan yet. If there are alluring "improvements" that don't work for such subtle reasons, this should be documented in the code. If it's not, one has only oneself to blame when someone goes in and changes it. Edit: I realise now that I'm talking about teams where everyone is reasonably senior. It could be different with junior members on the team, to which many changes might look like improvements when a senior engineer would at most see the change as equivalent. In that case I think you're right, but for a different reason: junior engineers should always check in with senior engineers about things in order to learn more of the craft!
- _pmf_ 7y ago
  > This sounds like the outcome of bad culture. For 90% of shops outside the cornucopia of SV with unlimited budgets and internal customers only, the client drives decision mercilessly and ruthlessly.
  - dx034 7y ago
    And for a good reason. The product doesn't exist because they're fun to develop. They exist because they solve a problem customers have. So ultimately, decisions should always be based on what customers (long-term) benefit most from.
mytailorisrich 7y ago
This isn't about rudeness or courtesy unless they are responsible for that code, in which case they should be involved or leading the change. As you mention it's about technical knowledge: They know exactly why the code is the way it is so consulting them is "due diligence".

Mirioron 7y ago

I wonder if we could have IDE features that will make #2 less likely. Eg marking some lines of code that they're similar to lines of code elsewhere. And if you then change it in one place the IDE will remind you about the others.

lonelappde 7y ago
Relying on your IDE to maintain program semantics is terrible. IDE should help you write good code that you commit, not be a crutch for writing bad code.
- Mirioron 7y ago
  People in the thread are arguing that it's not necessarily bad code though. Also, the point of the IDE would be to help you just not forget to change it.
hising 7y ago
JetBrains has something like this in their tools. It pointed me in the right direction a couple of times when touching code that is similar to something elsewhere. https://www.jetbrains.com/help/idea/analyzing-duplicates.html https://www.jetbrains.com/help/idea/analyzing-duplicates.htm...
- hising 7y ago
  Misunderstood the question (I just realized now ...). However. The tool in JetBrains is good at finding duplicates ...

JamesBarney 7y ago

I ask myself a couple of questions to decide when to abstract code to de duplicate it. 1. Is this domain logic or plumbing code?(for instance would this logic be described in a spec for the application) 2. How many times will these be duplicated? 3. How abstractable is it? 3.a. to abstract it will I have to do anything tricky like pass functions around use reflection? 3.b If I abstract this logic into a function is there a good concise name for it?

detaro 7y ago

And in the other direction, when adding flags etc to a method that was created to reduce duplication, consider if splitting it into two "duplicate" copies or copy it to the call-site requiring the flag isn't the better alternative.

danabramov 7y ago
Yes. If extracting and deduplicating is in your toolbox, so should be inlining and duplicating.
ahartmetz 7y ago
Or, if possible, split up the helper into smaller units of functionality that can be combined as appropriate for different requirements. That could possibly work for the original article's problem, too.

scarface74 7y ago

That can be boiled down to the “Rule of 3”. My CTO often asks me to implement a feature to do X and make it “generic enough to handle future use cases”. My answer is always the same - either give me at least three use cases now or I am going to make it work with this one use case. If we have another client that needs the feature in the future then we will revisit it. Of course, there are some features that we know in advance based on the industry how we can genericize it.

deleted 7y ago
[deleted]
rdiddly 7y ago
"Premature optimization something something..." ;)
- lolc 7y ago
  That's actually "premature generalization".
  - sverhagen 7y ago
    Isn't premature optimization a generalization of premature generalization? ;-)
RealDinosaur 7y ago
I think the catch all term for that is YAGNI.
- pytester 7y ago
  YAGNI is about not adding functionality until it's needed. DRYing code isn't adding functionality.
  - philipov 7y ago
    I think GP is implying the thing you're not going to need is a generic solution to the repetition. i.e: DRY is in fact a feature.

yodsanklai 7y ago

> Then you need to start passing options and configuration into your helper method... and before long your helper method is extremely difficult to reason about In which case, you should split the helper function ( extract sub-part common to all cases, and report the differences where the helper function is called). I think I would most of the time go with de-duplicating as early as possible, as long as the helper function only has few parameters and can be described in plain english rather easily. To me the cost of having to refactor the helper function later in the process is less than dealing with duplicated code. Duplicated code causes many issues. You mentioned introducing bugs, but it also makes the code harder to read. Every person who reads the code has to make the de-duplication effort mentally (check that the duplicated parts are indeed duplicated, figure out what they do, and on what parameters they differ...).

zmmmmm 7y ago
> Duplicated code causes many issues. You mentioned introducing bugs, but it also makes the code harder to read That is I believe contestable. Yes, it can end up easier to read but there is a big tradeoff - when you remove code from its context it is much harder for a reader to reason about it. Once something is extracted into a function you can't see, you have to mentally ask the questions like "could this return null / None?", "how will it behave if the input is negative?", "does it throw exceptions", "is it thread safe?" etc etc. All this is directly observable in context when the code is inline, not so when it is removed to a central place.
- yodsanklai 7y ago
  Ideally, these questions should be answered by the function name, type, and its documentation. And if not, one can always jump to the function definition and the above elements (missing if the piece of code is inlined) will give additional hints to what the code does.
- deleted 7y ago
  [deleted]
barrkel 7y ago
Premature optimization. Duplicated code is only evil when there's a bug you only fix in one place and forget about the duplicates; in almost every other case, it's easier to reason about and is more resilient in the face of local changes. Abstraction is often like compression, and compressed data is easier to corrupt. Change the implementation of an abstraction, and you put all consumers of it at risk. It's not an absolute good.

mfateev 7y ago

Go error handling is a good example of this. So far all the attempts to reduce the repetitive `if err != nil { ... }` through some abstraction failed. Look at https://github.com/golang/go/issues/32825 https://github.com/golang/go/issues/32825

asdfasgasdgasdg 7y ago
I don't really think Go error handling is a good example. A nil test paired with a return cannot be extracted to a function. A macro could perhaps centralize the logic, but otherwise it's no more amenable to deduplication than plain addition. Moreover, it's completely trivial, if verbose in the Go language. The case where it becomes interesting is when there are four or five statements that are repeated. If it's two statements, especially if the statements are both control flow, one of which is tied to the frame of the executing function, and the other is a trivial nil test, that falls firmly on the "not refactorable" side of the line.
renox 7y ago
In the language maybe but both Rust and Zig show that it's possible to have much less 'bloat' for error handling even without using exceptions. I'd say that go designers have still work to do: Zig especially show that you can be a 'simple' language and yet have both sane error handling and generics.
- arendtio 7y ago
  If someone else is looking for examples too, I found those: https://www.reddit.com/r/Zig/comments/99zlc9/exceptions_or_errors/ https://www.reddit.com/r/Zig/comments/99zlc9/exceptions_or_e...
  - DelightOne 7y ago
    On the error branch, how do you do code coverage?
  - marcus_holmes 7y ago
    I thought all of this until I got used to Go's error handling. There's a couple aspects to this: 1. After a while, the "if err != nil {" becomes a single statement in your mind, and you only notice it if it's different (like trapping things that should error with "if err == nil {"). In other words, it only feels verbose if you're not used to it. After a while, the regular rhythm of "statement, error check, statement, error check" becomes the routine pattern of your code and it looks weird if you don't check for errors (which is as it should be). 2. The point of Go's error handling is that it isn't magic. There's nothing special about error values, and they are handled exactly the same way as every other variable in the system. The only thing the language defines about errors is that they have a method called Error that returns a string. That's it. This means that you can create complex error handlers if you need it, entirely within the standard language. This is extremely powerful. The Go team's examination of the language error handling is interesting because it seems there's a conflict between newer Gophers who don't like the verbosity of it (but don't realise the power it brings) and the older Gophers who are used to the verbosity and appreciate the power. Almost exactly like TFA. The repitition looks ugly if you don't appreciate the reasons for it.

m0zg 7y ago

And a rule for Java programmers as well: nobody is going to "extend" your program. If your interface has only one implementation, you do not need the interface at that time, and possibly ever. Nor do you need DI or whatever other masturbation "best practices" Java gurus prescribe. When it comes down to it, Java is a simple, pleasant language. So people invent all sorts of indirection to appear smart. Don't. Just do it simply and readably, with as little indirection and abstraction as possible.

konschubert 7y ago

you can also try to reason about the situation to figure out if the duplication is incidental or inherent.

lilyball 7y ago

I have 2 rules I use when determining whether to duplicate code or to refactor: 1. How many duplications are there? If the code is duplicated once, that's fine. If it's duplicated twice (so 3 instances of it), then it's time to consider refactoring, subject to the next rule. 2. Why is the code duplicated? If it's "incidental duplication", i.e. code that happens to look the same, don't refactor. Only refactor if there's actually a reason why the code is the same. Which is to say, attempt to predict the future: if a change is made to one instance of the code, do I expect it to be replicated to the other instances too?

Hendrikto 7y ago
> attempt to predict the future Sounds very error prone…
- marcus_holmes 7y ago
  This. I wait until the feature is mature, and has been used by actual people. There's no point trying to clean up code that hasn't finished evolving, or that I don't understand fully. It also means I have a tidy stack of refactoring to do when I'm bored or need a quick motivational win :)
- lilyball 7y ago
  It is, which is why you want to be conservative. If the duplicated code is obviously supposed to be identical, then predicting the future should be trivial. If it's not obvious, then it's a question of "can I conceive of a reason why I'd want to update one and not the others?". And if the answer is unclear, wait a while and see if anything comes up.
jrumbut 7y ago
It's also useful to look at not just the duplication but the code itself. In this case, it was code for geometry which is not like to change all too much. Often the difference between harmless but ugly looking duplication and duplication that is actually harmful relies on the semantics of the code and not just its appearance.

daxfohl 7y ago

I call these "spaghetti abstractions", and they're the worst kind of spaghetti.

mhh__ 7y ago

Good generics can be really helpful here. Can't be specific without knowing the exact helper function but I find sitting down with a cuppa (away from the computer) and planning an interface (If it's C++) that accepts (say) a policy class with a default and a callable object ("Functor") very helpful. Sometimes it can't be done but it's better to have a bugfree building block (Macros don't count!) that can accept buggy user defined tasks than lots of repitition.

morelisp 7y ago
> Macros don't count! Well, C++ macros don't count.

skipants 7y ago

Agreed. And more importantly, (2) is waaaaaay easier to fix than the problem you outlined in your preamble.

babarock 7y ago

One of the areas where I really like "incidental duplication" is in tests. Tests can sometimes be very repetitive and identical, and it's tempting to want to refactor it in some clever way. That's almost never good. On top of the reasons laid out in parent comment, tests also function as unofficial documentation. I like having everything explicit in there, it makes them easier to read and understand.

lonelappde 7y ago
Terrible acronym for a good idea: DAMP stackoverflow.com/questions/6453235/what-does-damp-not-dry-mean-when-talking-about-unit-tests
blazespin 7y ago
Tests rarely have bugs, I find, so generally dry isn’t critical. Also, dry is for security (see below)
- lonelappde 7y ago
  Tests have fewer bugs if you write them before the system under test, and if they don't have mocks, and if you have enough of them that anything you get wrong the first time will get noticed by the results of the many similar tests.
  - gusmd 7y ago
    > Tests have fewer bugs ... if they don't have mocks 100x this. I've repeatedly fail to convince my team members that mocks are unnecessary in most cases. I've reviewed code with mocks for classes like BigDecimals and built-in arrays. This is especially prevalent in Java teams/codebases. Drives me insane.
- smnplk 7y ago
  As a preventative measure, I write some tests for my tests. Also in TDD style of course. And on a very rare occasion, I have to write a test for those tests as well.

BubRoss 7y ago

One big way I prevent this from happening is to treat classes as interfaces to data structures and keep everything that isn't about accessing the data elsewhere. Conversions to other data types go somewhere else. In fact I don't want my data types depending on any other data types at all. When doing this any of this repetition or evolution can stay out of the data structures themselves so that they can be reused without irrelevant baggage.

StevenWaterman 7y ago
Have you not simply abandoned OOP at that point? A core point of OOP is that objects manage their own state, and provide an interface for accessing/mutating it. If classes are only used as data structures, and everything is done through (presumably pure) utility methods, it sounds like you're writing procedural code in an OOP language. That's not inherently a bad thing, but OOP provides benefits and you may be making a trade-off without thinking about it.
- BubRoss 7y ago
  I try to worry much more about what works rather than labels. If you stuff all your functionality and data transformations into class definitions, your class definitions will be full of dependencies. Now all the fundamental elements of your program depend on each other and can't be separated. It's like trying to pile up concrete until it forms a cave instead of laying it down to make the floor and building on top of it. What am I missing?

c3534l 7y ago

Refactoring duplicated code is a trade off: on the one hand you create abstraction and centralizing code logic at the cost of, first, the overhead of learning that abstraction, and second, by increasing coupling between functions. I've personally found that the coupling is by far the most important factor to consider. If A depends on B, and C is found in both A and B, then you should factor out C. If A and B share C because they are adjacent, without being fundamentally intertwined, then duplicate C, but consider creating a library or module that makes it easy to talk about things that are similar to that duplicated code, C. (I tend to think about modules/libraries as little, independent DSLs)

soulofmischief 7y ago

Another thing you can do when a function becomes overencumbered is to split the remaining similar logic into smaller functions which are composed into specialized functions. This has benefit in that when analyzing modules, you can spot differences in procedure at a glance instead of needing to dig through 100 lines of somewhat similar imperative code.

loopz 7y ago
Right. There are two kinds of such refactoring / DRY-ing up the code: 1) Specialized helper sub-functions/classes to make the codeblocks DRY. 2) Functions/classes to make separate features DRY. Problem arises when the design or understanding of the code doesn't reflect the realities of changes over time, forcing you to restart/revert, or making spaghetti code with optionals and whatnot to accomodate the rising complexity. The agile approach would be to make the code that is easiest to change either way, and prevent being locked in to only one approach.

cheshire_cat 7y ago

Sandi Metz wrote a blog post about this: https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstracti... > The moral of this story? Don't get trapped by the sunk cost fallacy. If you find yourself passing parameters and adding conditional paths through shared code, the abstraction is incorrect. It may have been right to begin with, but that day has passed. Once an abstraction is proved wrong the best strategy is to re-introduce duplication and let it show you what's right. Although it occasionally makes sense to accumulate a few conditionals to gain insight into what's going on, you'll suffer less pain if you abandon the wrong abstraction sooner rather than later.

BalinKing 7y ago
Sandi Metz's blog (and book) are an absolute gold mine. I'm a junior developer (<5 years), and the material gave me an entirely new outlook on design (object-oriented and otherwise)... and also showed me how un-maintainably most development is done nowadays :( If there was a required reading list for professional developers, I would put her work on with zero hesitation, I feel it to be that important.
- bberrry 7y ago
  Which book are you referring to?
  - steveklabnik 7y ago
    Not your parent, but https://www.poodr.com/ https://www.poodr.com/
    - xenihn 7y ago
      She has a new book too. https://www.sandimetz.com/99bottles https://www.sandimetz.com/99bottles
- klyonrad 7y ago
  You call devs Junior until they have over 5 years experience? Wow, that’s harsh

SlowRobotAhead 7y ago

> I encourage my devs to follow a rule of thumb: don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. Let the code go through a few evolutions and waves of change. ... alright buddy, I’m going to need you to remove your spyware on my computer! I just did this last week with a CBOR message interpreter in C. I did it the long messy way, saw a bunch of repetition, “cleaned” it, changed the way the parser is called, changed some of the validator that runs after it... then did that entire process two more times as requirements changed. I’m near certain you wrote that ABOUT me! ;)

jdmg94 7y ago

inversion of control

wutbrodo 7y ago

My first job at a bigco had a very heavy read-the-code culture, and being able to read code and write readable code is one of the most valuable skills I learned there. The lack of this skill is one of the things I've found most frustrating about working with less talented or, now that I'm a bit later in my career, more junior engineers. There's a tendency to glom onto black-and-white, superficial rules without understanding them, instead of appreciating the principles underlying them and having a sense of how to balance trade-offs. This creates an unfortunate cycle: everyone writes unreadable code, so nobody practices reading code, so nobody internalizes what readable code looks like and continue to write bad code. I tend to have a strong reaction to duplicated code, but DRY is risky if whatever you're pulling out isn't logically concise and coherent[1]. Some of the helper functions I've seen in code reviews (as well as the one in the OP) strike me as written by unsophisticated language AIs, catching textual similarities that aren't semantically linked and pulling them out. The engineers I've mentored over the years, including ones starting with no eng experience, go on to write fantastic code (and no, this isn't just my opinion). But it's a very labor-intensive, hands-on process of thorough reviews and feedback until they internalize the underlying principles, and it can be tough to swing in very short-term-oriented company environments. Now that I'm running larger teams, I've been noodling over how to encapsulate this deeper understanding of what makes good code in a way that scales better. But the fact that Google et al haven't already done this makes me think it's not something you can teach with eg a bullet point list. > I encourage my devs to follow a rule of thumb: don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. Let the code go through a few evolutions and waves of change. (Note: this isn't a case of what I describe in the earlier part of my comment, as I don't think this is a superficial, black-and-white rule) I disagree pretty strongly here, especially since you're #2 involves waiting til you hit a bug. IME, there are many cases in which a solid understanding of the code allows pulling repeated code out in a way that's principled enough that it's more likely to adapt well to future changes. Indeed, this is true of most changelists to some degree, or you'd end up with no functions or classes other than main(). [1] A good rule of thumb is whether the free function has a reasonably readable name that captures its logic, without having to abuse handleFoo() or processBar().

useragent86 7y ago

slim 7y ago

the rule is "do the simplest thing that could possibly work"

hyperpallium 7y ago

Code duplication is a suggestion of meaning identity. But can be coincidential. Consider LZ78 (represents duplication with backreferences). Not very human readable.

ianamartin 7y ago

I think that people often make these choices based on latent social and work-related contextual factors as much as there is any misunderstanding or blind adherence to the wrong set of guidelines. There are human factors as well that go into people defensively citing guidelines for their decisions after a criticism comes up in code review. It's somewhat likely that the choice of a premature deduplication wasn't deeply considered. The programmer just did it because they felt like it made sense, and when questioned about it, they claim whatever "rule" exists to justify it. There's a strong pressure in work environments to get shit done, get it through review with as little fuss as possible, and move on to the next thing. That's work after all. Choosing to leave something in a state that's guaranteed to need attention later can be viewed as lazy. If you at least make an attempt to do something in a clean way, it will be perceived as "work" even if it turns out to be the non-optimal choice. I find for myself, at least, that I'm far more willing to let things evolve in my personal projects and to discover the right abstractions over time than I am in my work projects. I don't know if that's true for everyone though. My personal projects are always about the learning process. Maybe for a lot of people that's not the case? The focus is more on the product than the process? I write the first version of every side project in a language I don't know or don't know well and rewrite the actual version I'm going to use in a language I do know well. So I'm really happy to let things evolve, and the more I know the domain in one of my main languages, the more time I spend in the new language because I want to distance myself from what I think I know and examine it fresh when I come back to it and see if what I think I know is actually still (or was ever) valid. Different people have different teams and motivations both at work and at home, so while I agree with you that taking a wait-and-see approach is often a really good idea, there are often human factors involved in these decisions where the same person will make very different choices depending on their environments and motivations.

vbezhenar 7y ago

Another approach with statically compiled language and good IDE might be to reduce duplication whenever you see it and inline back when you feel that this abstraction is no longer useful.

watwut 7y ago

Extracting common function was right move in your example. The bad move was to add options to common functio instead of changing one caller to call new different method. Even worst move was add more and more options to originally simple method. Just because something was rightfully extracted to common place 10 months ago dies not mean it have to remain in common place. And the need for split does not mean original extraction was wrong.

vonseel 7y ago

I’m just wondering where you draw the line. As with everything, it often becomes a big grey area on what is acceptable and what is not. Example: my (fictional) company sells a B2B platform which provides companies with an online marketplace or some other type of online application. Each installation requires different, though often similar, integrations with the customer back-ends - think Postgres vs MySQL, but some customers may also use TotallyEffingObscureDB, which doesn’t even speak SQL. Those backends are usually the source of truth for the entire stack, storing user data, etc, the local data is just a mirror. So, given this scenario, how should we approach user registration processes? Is that a product component or custom (not DRY) code? What about all the other (similar, common, and basic) features of our platform that every customer wants but invariably needs to be implemented slightly differently? I’m only posing these questions because I worked in a situation where something similar happened, and it wasn’t dealt with well, and I’m wondering what other HN coders have to say...

Fanmade 7y ago
Ironically, we're handling this with the concept called "clean code". We do have a core, which does implement the base logic. Everything in there is domain driven, but only using DTOs and providing interfaces for input and output, using repositories and presenters. When the data source changes, we only need to add the new repositories and set those within the context. If we have to implement some specialized logic just for one client, we add this as a plug-in on top, so it is handled outside of the core logic. Of course all of this is very abstract and since I've worked mostly with simple MVC concepts until now, I'm still struggling to get my head around this approach, but so far it's looking very well. It may look overcomplicated at first, but there is a very strict while at the same time very flexible logic behind it, which can handle all the "creative" ideas our clients have so far, while still keep being maintainable. It is a long process to get to this method of programming, but my initial scepticism has completely changed to "why haven't I worked like this before?".
- qlk1123 7y ago
  Interesting. I'm curious how long did your core system with basic-logic take to reach that maturity, and how many people involved? What kind of development model did you use? Also you state that you "still struggling to get my head around this approach," does it mean that the system somehow violates the principle of least astonishment? (https://en.wikipedia.org/wiki/Principle_of_least_astonishment https://en.wikipedia.org/wiki/Principle_of_least_astonishmen... )

speedplane 7y ago

> don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first. I have a similar rule of thumb: when you do a thing the first time, just get it working. When you do a similar thing a second time, just get it working. When you do a thing a third time, pull it together as a cohesive abstraction. This is a silly generalization, but the point is that you generally need to see a handful of examples of a thing before you can make any useful abstraction. What's more common is people creating abstractions during the first iteration, leading to features that are never used, and others that were never considered.

sitkack 7y ago
And the abstractions are not abstracting over the correct things so they are actually a net negative. One has to reason about the abstraction AND the problem domain. I find it helpful to work with ADTs of values, lists and maps. No OO, just functions for selection and projection. The majority of programming is figuring out the nuances of the domain and getting something working. Code is actually an impediment to that.

b0rsuk 7y ago

From my experience, I would start with a more compact implementation first in hopes that it would save me time. This usually ends with me having to do more complicated operations in my head to plan the code. It's tempting. It's like trying to do math in your head to save time - often leads to a mistake. Now I believe there's no shame in trying out a solution first and rewriting it later when I get a feel what the problem is really about. John Carmack once said something like "If you're not sure which version is better, code it both ways and compare". The only downside of later optimization is that in commercial environment they often don't let you clean up.

_pmf_ 7y ago
> The only downside of later optimization is that in commercial environment they often don't let you clean up. Exactly (speaking for custom industrial automation projects). The follow up next requirements for a project might be in 5 or 7 years or never. You would be cleaning up a 80% dead project. In these environments, it's not uncommon to no longer have a dev environment because the system is a singular testbench that costs upwards of 2 million and only exists in 2 factories, so all bugfixes must be extremely conservative and any refactoring is stricly forbidden because there's no way to locally test for regressions.

gvjddbnvdrbv 7y ago

Reusing a function like this is not clean code. It violates several principals. 1. Functions should have as few parameters as possible and almost never have flag parameters. This is a basic thing and costs very little to follow. As soon as you want to add a flag to a function you need to make a new function. 2. Minimize coupling. 3. Single responsibility principal. A unit of code should have one reason to change. Of course in order to follow principles 2 and 3 here you may well need to consider the business logic.

partyboat1586 7y ago

I think about it in terms of bugs. If your abstraction causes a bug, I have to go in and work out wtf your wonky abstraction is doing and also risk breaking other cases. If there are 6 duplications and there is a bug because one of them is missing a change applied to all of them, that takes 5 mins to fix and risks breaking nothing. When you make an abstraction think not only "Will this create bugs?" But also "If this abstraction does create bugs will they be easy to identify and fix?".

truculent 7y ago

The best way I’ve had this explained is that refactoring != DRY. Some methods have a similar structure, but if the problem they’re solving isn’t fundamentally similar then it’s not truly refactoring and you’re setting yourself up for more work in the future.

jkoudys 7y ago

It's important for developers to understand that programming is at least as much about expressing yourself clearly with language, as it is about maths and compsci views of functions. Language that's more verbose, but also adds clarity, is a good thing. If I had an instruction book on building a cabinet, it wouldn't help to re-list every screw, tool, and their sizes on every single step if I could put a parts list at the front. But it also wouldn't help to collapse every matching group of steps with one or two different parameters together.

kostarelo 7y ago

Loved the "incidental duplication" term. I will def start using it from now on. But to add to this, I want to say that the moment you find your self writing a "helper" function, that's the moment when you must realize that you didn't understand the problem quite good. As a rule of thumb and what I am always trying to pass to my peers at work etc, is to always try and think in terms of your domain and the models in it. - What models does this helper function is acting upon or on behalf? - Does it make more sense to create a model and put that function in it? - Should that function be in a specific model or more that one?

kazinator 7y ago

Not only DRY but YAGNI. Unless you are sure you will need to separately evolve the initially duplicated cases, assume YAGNI: You're Not Gonna Need It (the repetitions). Avoid repetition like the plague; when the new requirements arise that conflict with the reduced repetition, confront it at that time. It's possible they they are bad requirements, in which case the clean, DRY code can support arguments against the requirements. Treat the computer as more of an Analytical Engine, and less of a Jacquard Loom. The special case behaviors for various shapes hinted at in the article: > For example, we later needed many special cases and behaviors for different handles on different shapes. This sounds like it might be a bad experience for the end user who has to learn annoying shape-specific handle quirks.

Shinchy 7y ago

I really like this way of looking at it.

avgDev 7y ago

I am working on a large project and I also realized this. I worked really hard to not repeat myself. I tried to reuse as much as possible as not to create extra code. Then, I quickly realized the things are NOT EXACTLY the same, and the methods to handle things were similar they were not the same. So, I had to refactor a lot of other helper methods. I agree with the approach you are describing. It would be easier to refactor later, as one would probably have a better understanding of the application in the wild.

collyw 7y ago

I am coming to the conclusion that code reuse is over pushed as an ideal in universities. Often the desire to create reusable code means that we create overly complicated code with less duplication.