Is copy and paste the head of for-loop (e.g.:for(let i=0;i<something.length;i++)) violating DRY principle?

Question

For example, in my project, I often found some head of for-loop appears many times, eg:

for(let i=0;i<SharedData.students.length;i++){
    SharedData.students[i].something=.....
}

if(isReset){
    for(let i=0;i<SharedData.students.length;i++){
        SharedData.students[i].reset();
    }
}
.
.
.

which the task that inside and outside for-loop are totally different, but it commonly needs

for(let i=0;i<SharedData.students.length;i++)

. So my question is, is copying and pasting

for(let i=0;i<SharedData.students.length;i++)

violating DRY principle?

Answer 1

The Don't Repeat Yourself (DRY) principle is easy to mindlessly over apply.

Keep in mind that the real sin isn't using copy and paste. It's spreading a design decision around in a way that makes it difficult to change that decision. If what you really have is two decisions that just happen to look the same at the moment then everything is fine. You'd be doing damage if you forced the two decisions to be expressed in the same place.

By leaving them as separate, as you have now, you're allowing the two loops to vary independently. If you rewrote them as Robert Harvey suggests:

for(let i=0;i<SharedData.students.length;i++){
    SharedData.students[i].something=.....
    if(isReset){
        SharedData.students[i].reset();
    }
}

then you'd lose the ability to easily make them vary independently (say by having one skip the first element, for whatever reason).

This idea can be hard to grasp so let me say it another way:

int x = 100;
int y = 100;    

Here is a "violation" of DRY that most people wouldn't think twice about. Why? Because we know that even though y is a redundant copy of x it might not always be. It has it's own meaning. We don't want to lose that meaning just because it happens to have the same value as x right now.

So please when you think about DRY think less about copy and paste and more about what you're making easy to change.

Answer 2

Is this violating the DRY principle?

To some degree, yes - and I think it is a little bit astonishing that some commenters here seem to overlook or deny it. Indeed, the consequences are often acceptable in lots of real-world cases, but I think it is worth to take a closer look at the example.

So let us assume for a moment this statement

for(let i=0;i<SharedData.students.length;i++){
    SharedData.students[i].something=.....

appears 100 times in a program. Then there are some design decisions which already became harder to change:

1. the decision to have SharedData an attribute named students

2. the decision that students is an indexable array

3. the decision that students has a mutable field named something

(Of course, you did not write to repeat the inner part of the loop, but let me put this into this example for the purpose of demonstration)

So how can you mitigate these issues? The first one can be mitigated by avoiding to repeat the explicit expression SharedData.students more often than necessary. Often, a simple additional local variable can help:

 let studentArray = SharedData.students;
 for(let i=0;i<studentArray.length;i++){
        studentArray.something=.....

Note that this simple change alone divides the number of repetitions of SharedData.students by two. On a larger scale, you may consider to have several functions implemented in terms of a parameter studentArray instead of a parameter SharedData.

Issue #2 can be mitigated, for example, by using a foreach statement, if your programming language has such a thing:

  foreach(student in studentArray){
       student.something = ...

Now, it is only necessary to have students an iterable container, which is a weaker assumption than being an indexable array.

Issue #3 can be attacked by encapsulating the inner part of the for loop inside a function:

foreach(student in studentArray)
      DoSomething(student);

Now, the logic of manipulating or using student in a specific manner is in one place, not 100 any more.

It maybe also worth to have a look why such a for-head repeats so often inside a program. It may be a sign that the overall code section containing the for loop can be generalized, maybe by introducing the operation as a parameter itself (I prefer C# syntax, I guess you get the idea):

 void DoSomethingForAllStudents(Action<Student> DoSomething)
 {
     foreach(student in studentArray)
         DoSomething(student);
 }

But beware, this can already be overengineered, and if the cost of making things less DRY is overengineering you should often better leave those things as they are.

As I wrote at the beginning, in lots of real-world cases the named issues are design decisions which you are not going to change later during the whole lifetime of your program, or where the real number of repetitions is not that high. So even if this literally violating DRY, don't overthink this.

Answer 3

Yes, this is violating the DRY principle. The DRY principle isn't absolute however and should be used with a personal standard. We don't live in a perfect world.

The frequency of use is a big tell however. If you are copy and pasting the exact same code frequently, I'd say that you should DRY it out.

Answer 4

First an other, more recognizable, example of repeated for-loops:

for (let i = 0; i < columnCount; ++i) {
    write getColumnTitle(i);
}
writeln();
for (let rowI = 0; rowI < rowCount; ++rowI) {
    row = rows.get(i);
    for (let i = 0; i < columnCount; ++i) {
        write row.getColumnData(i);
    }
    writeln();
}

It is easy to agree, that the loop is a bit verbose (i.e. the i), an iterator, or better a stream (passing a lambda function expression being repeatedly called) might be better style.

But in this case the DRY principle does not hold.

However if you see the following pattern:

for i A
B
for i C
D
for i E
F

Then the problem is not DRY but rather that the code is working generally working on entire i-tuples, records, but the loop handles a second dimension.

rec.a()
B
rec.c()
D
rec.e()
F

This code looks like a sequential processing, and is almost a nested loop for j for i X Y.

The most likely error here is a violated Separation of Concerns.

For instance importing from an Excel file into a database also has this two dimensional approach: reading Excel, writing database records.

The best practise is:

• make an Excel table reader, which can be passed:
  • a table definition
  • a row importing function
• make a row database writer
• make the integrating importer

Synopsis

1. The rec.a() approach of eliminating the for loops, is a step forward, but might cause too much complexity of many very specific, highly tied methods.

2. Separation-of-concerns might simplify things: also the above mentioned Excel reader might be developed in test-driven fashion in a unit test.

Answer 5

It depends.

The purpose of they DRY principal isn't so much to eliminate "identical looking" lines of code as it is to "facilitate consistency" and "reduce the burden of change."

Taking your example,

for(let i=0;i<SharedData.students.length;i++)

Under what circumstances would this line change? Are those circumstances "likely?" And, in the event of that change, would the identical or similar lines also change identically??

Use your judgement. Calibrate your own sensitivities over time.