I love copy & paste! I also defended it in this scholarly article: <a href="http://harmonia.cs.berkeley.edu/papers/toomim-linked-editing.pdf" rel="nofollow">http://harmonia.cs.berkeley.edu/papers/toomim-linked-editing...</a>
with a video: <a href="http://youtu.be/1wo_7MTdWWI" rel="nofollow">http://youtu.be/1wo_7MTdWWI</a>
<i>> This may come across as a straw man argument</i><p>Big time. The refactoring in this case was ill advised. When things started getting hairy, it should've been backed out.<p>Piling too much flexibility in one function is a common mistake. A justification for copy/paste it does not make.<p>I worked at a shop with this rule: don't try DRY until you've seen at least three repetitions. I think this saves one from premature refactoring.<p>Another way to put it: Refactor when the code speaks to you, that is when need is evident. Keep the result only if its a significant improvement. Avoid refactoring only because you are enamored of refactoring. (Or enamored of a rule.) Goes for any programming technique/tool, really.
That example under the <i>When Tools Make It Worse</i> section - uggghhhhh. Why would anyone actually <i>do</i> that? That isn't DRY refactoring, that's cargo cult refactoring.<p>DRY is not, was never, and should never be about unnecessarily replacing clean, well-factored code with @$2!% shared mutable state. The goal is to normalize your code, not to micro-optimize for keystroke count. No. Nonononononono. Just no.
Knowing when to refactor is obviously an art, not a science. And with an example as trivial as this, it's not really a very "real-world" example.<p>A lot of the time, you don't even <i>know</i> if two swaths of code are "coincidentally" identical (don't refactor) or identical in a "deep" way (refactor), even when the program is yours -- you just don't know how the program will evolve.<p>In the absence of additional information, I usually refactor only when I see <i>three</i> similar code paths, since by that point a project rarely goes back. Over the years, it's turned out to be a surprisingly good rule of thumb.
As I ponder this more, I think it's useful to consider the concepts of simplicity and complecting as articulated by Rich Hickey in his talk "Simple Made Easy". As he explains it, to complect is to braid multiple things together, whereas in a simple system, multiple things are composed. He has often pointed out that simplicity does not necessarily mean fewer things; as I understand it, it's not about how many things they are, but how they interact.<p>In that light, the single flexible tweet list function presented in this post is indeed problematic because it has a few things braided together: a tweet list, a profanity filter, and pagination.<p>So we should be suspicious of repetition, but at the same time avoid complecting.
One rule I try to follow is to avoid refactoring when the shared code is "coincidental." Perhaps this is another way of expressing what the author says about business logic.<p>I've definitely worked on projects where developers created large, unwieldy, hard-to-grok, buggy abstractions in the name of DRYing code. I'm pretty aggressive about making code DRY, but simplicity and readability are more important.<p>The effort I'll tolerate in pursuit of DRY also varies by language. I've been doing some Android work lately, and I'm finding that things I would have done DRY in Ruby require too much added complexity to make DRY in Java.
Straw man thinks DRY applies to two-line function. Straw man is a straw man. Also, less code > DRY. In fact, less code -> DRY. If refactoring makes for more code, not really DRY. More like taking a principle to its illogical conclusion. Compression is a process of diminishing returns.<p>Although there are certainly times when a factoring two lines into one line is better. Like when it's self-documenting, or when those lines otherwise add noise to part of another function.<p>Sometimes a new function is not the right approach to avoiding repetition. If you can't write a function to adhere to DRY, use a macro or equivalent. In C/etc, macros are wonderful if used well.
One of the things that people don't get about refactoring is that it is not just a matter of extracting things or removing duplication. Sometimes you merge things or re-introduce duplication to get someplace better.<p>When you look at refactoring examples online, they often make that mistake. There's a straight arrow toward a "better solution" but without any backtracking. It's a hobbled view of refactoring.<p>To bring it home, in the blog example, I think is perfectly fine to remove duplication in the way listed as "bad", as long as you reintroduce the duplication when you have a bit of trouble. Much of the time, you're lucky and you don't.
I really really like this take. Refactoring is usually pitched as something that is completely orthogonal to solving the actual problem you were given. I think too many of us (clearly, I'm projecting) are weary of anyone else going on a refactoring spree because we see it break down things that were just fine separate. Often with only "warm fuzzies" being the actual gain. The progression shown in this post is really really good.
I think the first step the author took on the refactoring path was one I wouldn't take. It breaks the "do one thing" rule and the rest of the post is the pain that naturally follows from having an over-generalized method that tries to do too much.
These articles are a dime a dozen. This popular philosophy isn't right, because look at my poorly coded example of it.<p>You're dry code, is only dry is the laziest of senses, and represents a lousy programmer cluttering the system. A really lousy implementation of any of these programming paradigms would make one side look wrong.<p>In your example, the refactored code would look excellent if it implemented OOP and the Strategy Pattern. The two different feeds can inherit their similarities from the same place, and their differences implemented in separate places. Which feed to produce can be chosen dynamically, rather than one crappy grab-all function.
Not directly related - but it's something I come across so often when mentoring newbie devs that I thought I'd mention it in passing just in case anybody has this problem.<p>A pattern I sometimes see with newbies who understand the value of DRY is - as soon as they get to the point when they're about to repeat something or about to copy and paste - they stop themselves and start refactoring to remove the duplication they haven't typed into existence yet. They see adding the code that will produce the duplication as bad / waste.<p>Don't do that.<p>It's hard - because the code that they've not typed or copy/pasted doesn't exist or work yet. It's still in their head.<p>Make the duplication explicit first.<p>Type it out. Copy and paste. Change those two branches so they have exactly the same structure.<p>When you've done that - and everything is working and all tests pass - then refactor the heck out of it.<p>Much simpler, faster and less error prone.
I expect most of us would agree that a single function should ideally have exactly one main job and do it well.<p>Two functions are really doing the same job, and should probably therefore be combined into a single function, not when their behaviour <i>is</i> the same but when it <i>should always be</i> the same. As the article suggests, that determination is generally more about the software design or domain model than the mechanics of the current implementations.<p>Having said that, there is also a middle ground: create some sort of utility/helper function(s) to contain the code that <i>is</i> the same, coincidentally or otherwise, and then rewrite the two higher-level functions in terms of common helpers for now. If those higher-level functions need to diverge for good reasons later, at least it will be an active decision to separate the behaviours.<p>IME that sort of breakdown is unlikely to be beneficial with very short functions such as the examples here. There’s not enough commonality to justify the overheads of breaking everything up. However, in more realistic code, if you’ve got, say, 80% common operations between multiple cases, there are often some underlying concepts that can be extracted into their own functions. Those then become informatively named building blocks for the original functions.<p>Put another way, you might not want to consolidate the functions’ <i>interfaces</i> if they serve logically distinct purposes, but you can still consolidate some of their <i>implementation</i> details.
I see a lot of comments here of the type "You have to know when to refactor". I don't do it this way. Instead, I rely on a willingness to undo a refactoring when I see that something else might work better -- and even to undo <i>that</i> when I decide that I've got that wrong.<p>I have no problem extracting as in "WHEN REFACTORING GOES BAD" -- although I might wait for a third copy because removing the duplication -- because I want to see whether a useful abstraction would emerge. On the other hand, as soon as I recognise that one of those copies wants to change in a way that the other does not, I'd simply inline the method and let them diverge. I don't consider this a problem.<p>It seems as though some programmers believe that, one they extract something, it needs to remain extracted. No. It's only "cargo cult refactoring" if you stop thinking.<p>Most importantly, refactoring is experimentation. It's a kind of Mechanical Turk-based genetic programming-oriented style of designing, except that you have heuristics you can follow. That means that you'll go down the wrong path. THAT'S OK! as long as you allow yourself to backtrack. Remember: refactorings are small, <i>reversible</i> design changes. That means not just that one <i>can</i> undo them, but that one is <i>willing</i> to undo them.
OK, I'm obviously missing something, and part of the problem is that I'm not a Python programmer so my brain is obviously in "skim-mode".<p>Couldn't the problematic DRY pattern be alleviated by refactoring the following call:<p><pre><code> filter_profanity = kwargs.pop('filter_profanity')
tweets = Tweet.objects.filter(**kwargs)
if filter_profanity:
tweets = itertools.ifilter(lambda t: not t.is_profane(), tweets)
return render(request, template, {'tweets': tweets})
</code></pre>
Into something like:<p><pre><code> def tweet_list(request, **kwargs)
...
tweets = get_filtered_tweets(kwargs)
...
def get_filtered_tweets(**args)
filter_profanity = args.pop('filter_profanity')
if filter_profanity
etc....
end
return tweets
end
</code></pre>
Why does the logic for the Tweet filtering have to be encapsulated in the rendering function?<p>// edit:<p>What might help is if the OP showed how the non-refactored code would look with the profanity_filter and pagination features. I agree that his refactored proposal is confusing...I'm just having a hard time imagining how the non-refactored version would be less so.
My only real concern with this essay is that the OP bothered to refactor out the duplication, but didn't bother to refactor his internal refactoring when it got too complicated, instead claiming: "look, now it got messy", threw up his arms, and said there's nothing more that can be done, blaming DRY as the culprit.<p>Except we CAN do something about it.<p>It would have been just as easy to continue refactoring the tweet_list() method to pull filtering, pagination, and profanity checking out into sub methods-- at which point you've built a strong reusable component that can support many more combinations of those extra requirements. So by the time you get more feedback saying, "we need a new page that only shows 5 tweets per page and hides profanity, but does not filter", you can now easily take that reusable component, pass in those options and be done rather than starting from the top because you refused to clean up your internals. That's why we strive for reusable components in the first place.<p>In other words, if the argument is that refactored code is messy, it really means you aren't done refactoring.
Refactoring / DRY to me is not about creating monolithic, generic do anything functions. It decomposing code into layers of abstraction somewhat like mini-"DSL"s. The top level functions are tying together next level "down" helper functions. Which may themselves be higher level tools over something like DB api. More than 3-4 layers is probably a smell.
In defense of the single flexible function, I think the hypothetical business requirements are pathological. Or perhaps the hypothetical developer is taking a pathologically literal interpretation of them. Who would want pagination in one view but not another? As for the profanity filter, that should probably be a preference of the currently logged-in user which is applied to all feeds which that user views. (It should probably be enabled when an anonymous user is viewing any feed.)<p>I suppose some developers don't have the freedom of suggesting alternative specified behavior that is nicer to implement. In some cases I have not had that freedom. But in this hypothetical case, when pressed, the person setting the requirements ought to value consistency.<p>My own experience has been that I tend to do copy-and-paste because it's easier, but then regret it later. I don't think I've yet erred too far on the side of trying to follow the DRY principle.
I did a self-study on a project that lasted several months. I wrote down everything, including mis-typed characters, grammar errors, syntax errors, and semantic errors. I did a root cause analysis of the result. One general result is that I have a 3% error rate, regardless of activity. In fact I find that the delete key is by far the most important key on the keyboard, representing 3% of all of the characters I type.<p>One observation is the fully 1/2 of ALL the programming errors I made were due to copy/paste. Your mileage may vary but I doubt it.<p>Copy/paste is evil but it is so "low level", like the delete key, that you probably don't even think about it.<p>You may find it worthwhile to do a deep analysis of your personal error rate on some project. It is very enlightening. In fact, we ought to fund studies so we can get industry wide statistics.
The thing is:<p>"If you are using copy and paste while coding you are probably committing a design error" doesn't conflict at all with what he says. The fact is that copy and paste is the point when one looks and says "is refactoring appropriate here?"<p>One thing I would point out is that premature optimization is the root of all evil. You can get a pretty good sense that if your refactor adds more lines than it deletes and functionality remains the same, that you have added complexity in refactoring which means very likely that you are doing it wrong. This is particularly true if you can't say it is reducing the number of lines of code generally, or compartmentalizing state changes.<p>(This leaves aside the fact that the most pernicious use of copy and paste in the world is "sample code.")
If your code is starting to look ugly, refactor it. If your refactoring is looking ugly, stop, you're doing it wrong. If a test breaks because of your refactoring, stop, you're doing it wrong. I call this the Don't be Stupid principal.
Isn't this what functional programming is for?<p>You have several pieces of code that follow a very similar structure and logic but perform very different purposes for the program. So you try and generalise the structure of the code?
Refactor if doing so will give you an advantage.<p>Copy paste when you aren't sure if the requirements will change. Nothing is worse than building an abstraction only to find out it's useless given this new project requirement and that the two abstractions should really be separate.