2

I have the following code all over my project and I would like to know if it is possible to write the same in less lines of code by replacing it with a single for loop that increments or decrements depending on whether endPosition is bigger or smaller than startPosition. Is that possible?

if (endPosition > startPosition) {
    for (int i = startPosition; i <= endPosition; i++) {
        doStuff(i);
    }
} else {
    for (int i = startPosition; i >= endPosition; i--) {
        doStuff(i);
    }
}

Edit1: I have changed the words 'optimize' to 'write in less lines of code'. Also, I added the index i as a parameter of doStuff, to emphasize that it's important. The order in which each element is visited, however, is not.

Lucas Mendonca
  • 387
  • 3
  • 13

9 Answers9

1
int step = endPosition > startPosition ? 1 : -1;
for (int i = startPosition ; (i-step) != endPosition; i += step) {
  doStuff(i);
}

Note that this will always execute doStuff at least once (i.e. startPosition and endPosition are considered inclusive).

Joachim Sauer
  • 302,674
  • 57
  • 556
  • 614
1

You use the word "optimizing" for this but what you want is not an optimization. It is always better to have more code which is easier to read and understand than to have fewer lines of code that will make your team members scratch their heads trying to understand it. Anyway, just as a puzzle, if you want to keep the code to a minimum and not use any if:

int max = (int)(((long)startPosition+endPosition) + Math.abs((long)startPosition - endPosition)) / 2;
int min = (int)(((long)startPosition+endPosition) - Math.abs((long)startPosition - endPosition)) / 2;
for (int i = min; i <= max; i++) {
    System.out.println(i);
}

Here's how I got to this. I started with:

int step = (endPosition - startPosition) / Math.abs(endPosition - startPosition);
for (int i = startPosition; i != endPosition+step; i+=step) {
       System.out.println(i);
}

Then got a nice comment: what if endPosition=Integer.MIN_VALUE and startPosition>=0? We expect to have the step=-1 in this case but what happens is (endPosition - startPosition) won't fit in the size allocated to an int. This means that the first bit of the int, the one giving the sign of the number, will be overwritten. And so instead of being 1 as we expect it for the negative value -1, it will be 0. So step=1. To address this I would cast to long and then back to int:

int step = (int)(((long)endPosition - startPosition) / Math.abs((long)endPosition - startPosition));

Still not out of the woods yet: if endPosition is equal to startPosition we get division by zero. So... better get the min and max mathematically. And also we want to keep in mind cases when the sum or difference won't fit in int type.

wi2ard
  • 1,471
  • 13
  • 24
0

It can be reduced to a single loop with ternary condition operator, but I think it's quite ugly:

boolean up = endPosition > startPosition;
for (int i = startPosition; up ? i <= endPosition : i >= endPosition; up ? i++ : i--) {
   doStuff();
}

Of course the endPosition > startPosition condition can be inlined to eliminate the up variable, but that would be even uglier in my opinion.

Eran
  • 387,369
  • 54
  • 702
  • 768
0

If you don't need to use indexes in the loop, you can replace your code with next:

int k = Math.abs(endPosition - startPosition);
for (int i = 0; i <= k; i++) {
    doStuff();
}
Egor
  • 1,334
  • 8
  • 22
0

Your question is not related to cpu optimization. You are asking for less lines of code, it is different.

If order is not important, yo can do (but your version is better at cpu):

int[] limit = {Math.min(start, end), Math.max(start,end)};
for (int i=limit[0]; i<limit[1]; i++) {
  doStuff();
}

If order is important, you can use any of the other answers... but your code is actually better at cpu anyway.

mnesarco
  • 2,619
  • 23
  • 31
0

One could make one loop of it, but is wieldy code with code repetition. A lambda would make sense.

walk(startPosition, endPosition, i -> { doStuff(); return false; });

Or with

boolean doStuff2(int i) { return i == 42; }

walk(startPosition, endPosition,  this::doStuff2);

With:

/**
 * @param step function that receives the index, and returns false to continue.
 * @return the index of a return true, otherwise -1.
 */
int walk(IntPredicate step, int startPosition, int endPosition) {
    if (endPosition > startPosition) {
        for (int i = startPosition; i <= endPosition; i++) {
           if (step.test(i)) {
              return i;
           }
        }
    } else {
        for (int i = startPosition; i >= endPosition; i--) {
           if (step.test(i)) {
              return i;
           }
        }
    }
    return -1;
}

Cleanest:

static IntStream walk(int startPosition, int endPosition) {
    return startPosition <= endPosition
        ? IntStream.rangeClosed(startPosition, endPosition)
        : IntStream.rangeClosed(-startPosition, -endPosition)
                .map(i -> -i);
}

    walk(1, 4).forEach(System.out::println);
    walk(8, 4).forEach(System.out::println);
Joop Eggen
  • 107,315
  • 7
  • 83
  • 138
0

like this:

for(int i = Math.abs(endPosition - startPosition); i >= 0; i--) {
    doStuff();
}
Tom
  • 4,096
  • 2
  • 24
  • 38
0

If performance is you main concern, the way you wrote it in your question is the way to go; however, if the pattern appears several times in you code, and to avoid repeating the code in the loops, you can do something like this:

public static void forEachInRange(int startPosition, int endPosition,
        IntConsumer cons) {
    if (endPosition > startPosition) {
        for (int i = startPosition; i <= endPosition; i++) {
           cons.accept(i);
        }
    } else {
        for (int i = startPosition; i >= endPosition; i--) {
           cons.accept(i);
        }
    }
}
Maurice Perry
  • 9,261
  • 2
  • 12
  • 24
0

I find the problem amusing but really I would never implement something like this, too prone to infinite loop on edge cases....

But just for fun, you can use the compare function, and implement the code without any conditionals.

        int start ;
        int step;
        int end;

        start = 3;
        end = 9;
        step = Integer.compare(end, start);

        System.out.println("==========");
        for(int i = start; i!=end+step;i+=step){
            System.out.println(i);
        }


        end = 3;
        start = 9;
        step = Integer.compare(end, start);
        System.out.println("==========");
        for(int i = start; i!=end+step; i+=step){
            System.out.println(i);
        }

minus
  • 2,646
  • 15
  • 18