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.

Improve this question
6
  • 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
    Commented Sep 7, 2015 at 0: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
    Commented Sep 7, 2015 at 0: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.
    – user113093
    Commented Sep 7, 2015 at 0:38
  • @WinstonEwert I will look into that. I am not sure I want to do that for all C#/VB.NET types, though.
    – user113093
    Commented Sep 7, 2015 at 0: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
    Commented Sep 7, 2015 at 8:41

1 Answer 1

Reset to default
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.

Improve this answer
2
  • 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.
    – user22815
    Commented Sep 7, 2015 at 6: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
    Commented Sep 12, 2015 at 1:02