0

I have a block of code that branches into 2 pathways, let's call them the "simple" and "complex" branch, based on user input.

Either the simple or complex logic has 4 steps, let's call them A, B, C and D. A, B and C each have 2 different variations depending on whether we're in the simple or complex branch.

In the complex case, A, B and C are long enough where it makes sense for them to be broken up into their own methods. In the simple case, these reduce to nearly 2-3 liners.

Because the simple and complex case correspond to different user inputs, I feel it's easier and less mentally taxing to think about the problem with 1 if statement branching into 2 different methods, rather than 1 method with multiple if statements inside. This way, when I look at the methods of either branch, I'm only having to ever think about the logic of 1 case at the time rather than look in and out of if statements to see the program flow.

Now the dilemma is that both methods share identical code for D logic, and while they correspond to different user inputs, D is unlikely to ever vary between them. In the complex method, A, B and C are broken out into separate subfunctions, so it is logcally consistent for D, which is the same logical level as A B and C, to also be broken out into a separate function. In-lining, in this case, hurts readability.

However, in the simple case, A, B and C are all in-lined into the simple() method. Each of these steps is short enough that any separation into subfunctions would hurt readability (and, since D represents a similar level of step in the process, it would be logically inconsistent to break D into a sub method).

I'm currently most inclined to have D in-lined in the simple case, so that A, B, C and D can be read in one function in simple(), and then have identical logic for D broken out into a sub-function in the complex case. This violates DRY, but I feel this improves readability.

In summary, in pseudocode:

if(condition)
  simple()
else
  complex()

function simple() //All in-lined in simple()
 Asimple;
 Bsimple;
 Csimple;
 D;

function complex()
  Acomplex();
  Bcomplex();
  Ccomplex();
  D();

D is copy-pasted in D() and in-lined in simple(), but I believe this is more readable as both simple and complex have their internal logic at a consistent level of decomposition. And, by keeping logic all together in the simple case and decomposed on the complex case, this maximizess readability by keeping functions neither too short nor too long. Does this seem like an appropriate time to violate DRY?

  • 2
    DRY is not an end in itself. Don't overemphasize organizational consistency. It's more important to consider if both versions of D will have to be changed in the same way (if every change will have to be done in two places). If that's something you can already tell or reasonably expect, then it's better to DRY out the code and have one D() method. If they are really going to evolve independently or if you'll end up with a bunch of if statements in that method, then it's better to split them. Another option to consider is object polymorphism in place of the two functions. – Filip Milovanović May 19 '20 at 4:53
  • 1
    @Filip Milovanović why not post it as an answer? – Rik D May 19 '20 at 5:46
2

When to introduce functions

You're looking at that question from a code-centric view, deciding how to organize it mainly from the "repetition" viewpoint. When I started programming back in the 1970s, we were taught that subroutines are to be introduced if you find yourself repeating the same sequence of instructions in multiple places. The DRY principle is still (to some extent) in line with that approach.

But I'd stress a different point of view: functions (or methods/subroutines, whatever you want to call it) as abstractions for some computational tasks.

By introducing an appropriate set of functions, you create the language you want to use for expressing your algorithm.

Back to your example

  • In the complex case, you already decided to use a language where the steps A to D are expressed in one word, i.e. a call to Acomplex().
  • In the simple case, you seem to prefer to not use abstract names for the corresponding steps, but to use a more basic language without the A to D abstraction. And I find that an inconsistent coding style.

As in both cases that task is composed from 4 clearly-defined consecutive steps, I'd prefer to see that reflected in the code in both cases. So, I'd recommend 7 functions (you'll surely come up with better prefixes than A to D):

  • Asimple()
  • Acomplex()
  • Bsimple()
  • Bcomplex()
  • Csimple()
  • Ccomplex()
  • Dcommon()

This way, you make it clear to future readers of your code that the two cases share the same overall structure, but differ in implementation of some steps (visible in the simple/complex/common name suffixes). And to me it doesn't matter if some of these functions can be implemented by a single line of code.

What about DRY?

Viewing the DRY principle from a language-creation point of view, it basically says that

  • You shouldn't repeatedly use lengthy phrases that always convey the same concept (repeat the same code fragments over and over again if they serve the same purpose), but instead create a shorter name, as we use the term "car" instead of "four-wheel vehicle with combustion engine meant to be used on well-paved roads".
  • And you shouldn't introduce multiple terms for the same concept.

P.S. Of course, polymorphism will even be a better option if that fits into your architecture here.

1

When you read about DRY, there is no absolute prohibition of repetition, you have to look at the cost.

If the same calculation is performed in three different places miles apart, and the calculation needs to change, you can be sure that at most two of the three places will be found and you have a bug.

If the same code is executed in two consecutive methods, that appear on the same screen, that risk is much much lower. And that is your situation.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.