35

I have some code that is nearly identical, but uses absolutely different types, with no inheritance between them, on the main variable. Specifically, I am writing an analyzer with Roslyn for C# and VB.NET, with the following types:

Microsoft.CodeAnalysis.CSharp.Syntax.AttributeSyntax Microsoft.CodeAnalysis.VisualBasic.Syntax.AttributeSyntax

I am wondering if, because the code is doing the same thing, I should keep it as DRY as possible, splitting off as little as possible into separate (but identical other than the type) methods, or completely separate it because the two methods are not related and future changes could force one version to change, but not the other (although this is unlikely)?

Edit: A year or so later, I hit this same issue, and the Roslyn team helped me solve it: Write a base class that takes generics and has a TAttributeSyntax parameter that does most of the work. Then, write derived classes with the bare minimum of data that needs a specific type.

  • Would it work to create your own AttributeSyntax interface that wraps the existing classes but gives you the inheritance which conceptually should be there? – Winston Ewert Sep 07 '15 at 00:26
  • 7
    Sorry if this is obvious, but generics exist so you don’t have to repeat yourself for code that’s identical but for type. If that’s not what you meant, please disregard. – Davislor Sep 07 '15 at 00:35
  • @Lorehead Typically I would do that, but this is a single method being passed a type which contains the node as a payload from an internal method I have no control over. –  Sep 07 '15 at 00:38
  • @WinstonEwert I will look into that. I am not sure I want to do that for all C#/VB.NET types, though. –  Sep 07 '15 at 00:39
  • Okay, thought there was probably a reason. – Davislor Sep 07 '15 at 00:39
  • 1
    Refactoring imposes many compromises and sometimes even paradoxons. E.g. w.r.t. loose coupling vs. DRY, or short functions, but many thereof, vs. longer functions, and few thereof. In the end, it's a difficult beast: Your target is readability and maintainability. You need to think as an avatar that sees your code for the first time. And sometimes you just try out to _see_ what's better. Perfect refactoring unfortunately is not possible. – phresnel Sep 07 '15 at 08:41

1 Answers1

111

You don't do DRY because someone wrote it in a book somewhere that it's good to do, you do DRY because it actually has tangible benefits.

Specifically from that question:

If you repeat yourself, you can create maintainability issues. If doStuff1-3 all have similarly structured code and you fix a problem in one, you could easily forget to fix the problem in other places. Also, if you have to add a new case to handle, you can simply pass different parameters into one function rather than copy-pasting all over the place.

However, DRY is often taken to an extreme by clever programmers. Sometimes to not repeat yourself you have to create abstractions so obtuse that your teammates cannot follow them. Sometimes the structure of two things is only vaguely similar but different enough. If doStuff1-4 are different enough such that refactoring them to not repeat yourself causes you to have to write unnatural code or undergo clever coding backflips that will cause your team to glare at you, then it may be ok to repeat yourself. I've bent over backwards to not repeat myself a couple of times in unnatural ways and regretted the end product.

So, basically, don't think "oh man, this code is pretty similar, maybe I should refactor to not repeat myself". Think "does refactoring to make this code base reuse common elements make the code more maintainable or less maintainable?" Then, pick the one that makes it more maintainable.

That being said, given SRP and just trying to have small, flexible classes generally, it might make sense to analyze your code for that reason, break apart bits of behavior that use generic types (you said that they are identical other than type) into small classes. Then you'll find out that some of these classes actually are totally identical (not just mostly identical), and then you can build a toolkit in case you want to add Microsoft.CodeAnalysis.CPlusPlus.Syntax.AttributeSyntax.

Community
  • 1
durron597
  • 7,590
  • 9
  • 37
  • 67
  • 32
    TL;DR - DRY is a means to an end. Focus on the end, not the means. If I could upvote the Lego Man twice, I would. –  Sep 07 '15 at 06:02
  • One important note: if you _do_ repeat yourself, always mention in the comment all the other places that have to be visited when the repeated code changes. Not only it reduces the likelihood of a desync, it also works as an indicator of how much maintenance pain the repetition is causing you. – Xion Sep 12 '15 at 01:02