Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I think the Dont-Repeat-Yourself mantra is a bit overused. For example, assume you have a bunch of functions that all have the following code sequence

    A()
    B()
    C()
So someone thinks, hey, lets extract that bit, and create a function that does all three things

    def ABC():
      A()
      B()
      C()
Now the calls site is much simpler and there is no more repetition:

    ABC()
Except you realise that in some call sites the B() call was missing, so to fix that you add an argument:

    def ABC(doB):
      A()
      if(doB): 
        B()
      C()
The call site now looks a bit less clean, but still no repetition!

    ABC(True)
or

    ABC(False)
Then you realise in some cases the C() call is not necessary, so just add another argument:

    def ABC(doB, doC):
       A()
       if (doB):
          B()
       if (doC):
          C()
And your call sites look like this:

    ABC(True, False)
etc.

At that point the repetition in the original is definitely preferrable...



If you put names to those functions it instantly feels a lot less like 'the original is definitely preferrable', for example something like:

    def handleNewEmail(isInboxDisplayed, areNotificationsEnabled):
      AddToInbox();
      if (isInboxDisplayed):
        RefreshInboxView();
      if (areNotificationsEnabled):
        SendSystemNotification();
Not defending needless refactoring in all cases, it's definitely a judgement call.

(Edit: formatting)


In your example the if statements would be needed in any case, since they depend on external state. In that case the refactoring would make sense.

I'm talking about the case where the conditionals are only needed because of the refactoring.


Ah I presumed from your example that conditionals were external to the doABC() logic since they were subsequently passed in as params?

To be honest, I find it hard to discuss readibility without a real-world example. The algebraic placeholders feel too terse.


I'm talking about the case where you have these two functions:

    def func1():
      ...
      A()
      B()
      C()
      ...  

    def func2():
      ...
      A()
      B()
      C()
      ...
Which is then refactored to

    def func1():
      ...
      ABC()
      ...  

    def func2():
      ...
      ABC()
      ...  

    def ABC():
      A()
      B()
      C()
Which is sensible, but then other functions show up:

    def func3()
      ...
      A()
      C()
      ...
And you refactor to:

    def func1():
      ...
      ABC(True)
      ...  

    def func2():
      ...
      ABC(True)
      ...  

    def func3()
      ...
      ABC(False)
      ...

    def ABC(doB):
      A()
      if (doB):
        B()
      C()
I'll leave the final example to your imagination, since I've already used so much screen space.

I'm currently in the process of undoing a lot of such refactorings in my codebase because it has become unmanageable.

I really think repetitive code is much easier to work with than overabstracted code.


I think the main problem is lack of expressiveness (what does True mean?). Since your examples seem to be in Python, I would solve it there with named parameters, maybe even giving them default values. That way the code may be abstract, but it is also informative.


I used to share the same example against obsession with DRY. But it's a bit more nuanced.

Both options can be valid. DRY is a tangential topic, the main goal should be to keep your code as close to your mental model as possible (also keeping mental models in sync between team members, which is quite hard).

ABC being a sequence could be a pattern, or it could be a coincidence. You can't know which one it is just by looking at the code. The knowledge whether it's a sequence or not is in the business domain and in your mental model of that domain.

You might think you're disagreeing on DRY with another team member, but in reality you two have different mental models, and one of you is using DRY to justify a his.


Very well put. I run into this all the time in the name of "reducing complexity", when it is really hiding it under the rug.

My personal approach to combat this is better data structures that model the problem (and are checked by the compiler). Once this is in place I try to "flatten" the calls, so that things are mostly at the top level, or few levels deep, which usually comes up naturally once the data structure is consciously defined.

I try to have as much code as possible be "structure in - structure out" (pure functions) and to concentrate stateful code to work on the structure's fields/values. This is surprisingly easy once the data structures match the problem, instead of only growing organically.


A codebase is a living thing. Inlining a function or splitting it into multiple cases should always be an option, and boolean flags are generally a code smell. I don't see this as an argument against DRY; when the facts change, your code structure needs to change too, but that doesn't mean your original structure was wrong.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: