Reputation: 173
I have been experimenting with a game that I'm trying to make. I found I had two methods that were identical except for the for loop, which was simply the reverse of the former.
I tried to make it so I can use the same code forwards or backwards. I have ended up with:
for (int i = start; i != (finish + 1 * ((start < finish) ? 1 : -1)); i += 1 * ((start < finish) ? 1 : -1))
A) B) I wanted to share this concept with everyone.
B) I'm curious about efficiencies of such a loop. I know it is calculating the target number and the incriment factor when accessed, but I have no idea how to test it.
C) While I'm at it, I found a single line that swaps two variables without using a temporary. It takes a moment to read, but is this better (code wise) than using a temp? (3rd line in main below)
I have tested it for function, and it runs as expected. If the second number is larger than the first number, it counts up. If not, it counts down. My test code was:
// A generic loop that can go up or down
import java.io.*;
public class reversable
{
public static int start = 1;
public static int finish = 10;
public static void main(String[] args)
{
for (int i = start; i != (finish + 1 * ((start < finish) ? 1 : -1)); i += 1 * ((start < finish) ? 1 : -1))
{
System.out.println("i = " + i);
}
finish = (start + finish) - (start = finish);
System.out.println("Finish = " + finish);
System.out.println("Start = " + start);
for (int i = start; i != (finish + 1 * ((start < finish) ? 1 : -1)); i += 1 * ((start < finish) ? 1 : -1))
{
System.out.println("i = " + i);
}
finish = 10;
for (int i = start; i != (finish + 1 * ((start < finish) ? 1 : -1)); i += 1 * ((start < finish) ? 1 : -1))
{
System.out.println("i = " + i);
}
}
}
Based on comments, would this be acceptable:
public static void reversable (int i, int j)
{
if (i > j) int inc = -1; // Count down
else int inc = 1; // Count up
j += inc;
for (i; i != j; i += inc)
{
dostuff();
morestuff();
mostuff();
}
}
Upvotes: 0
Views: 319
Reputation: 53616
Indeed the code is hard to follow... but enough with the critics. When someone ask for reusable methods where only the loop is different, the only thing that comes to mind is an Iterator
. The iterator pattern is exactly what you need for this reusable case. If your wrap your iterator in an Iterable interface, then you can easily use it inside a for
block. Example :
public class IntegerRange implements Iterable<Integer> {
private boolean reverse;
private int start;
private int end;
public IntegerRange(int start, int end) {
this.reverse = (start > end);
this.start = start;
this.end = end;
}
@Override
public Iterator<Integer> iterator() {
return new IntegerIterator();
}
private class IntegerIterator implements Iterator<Integer> {
private int current;
private IntegerIterator() {
current = start;
}
@Override
public boolean hasNext() {
if (reverse) {
return (end <= current);
} else {
return (current <= end);
}
}
@Override
public Integer next() {
if (!hasNext()) {
throw new NoSuchElementException();
}
if (reverse) {
return current--;
} else {
return current++;
}
}
@Override
public void remove() {
throw new UnsupportedOperationException("Cannot remove from this iterator");
}
}
}
Then use and reuse your iterator...
static public void main(String...args) {
doStuff(new IntegerRange(1, 10));
doStuff(new IntegerRange(10, 1));
}
static private void doStuff(IntegerRange range) {
for (int i : range) {
System.out.println("i = " + i);
}
}
Code more readable now.
Upvotes: 0
Reputation: 406105
If I saw this code in production I would immediately refactor it in to something more readable, which would probably be something like what you started with.
Think about the reason you want to reduce code duplication. It's to make your code more maintainable. Do you feel like you've done that?
While I'm at it, I found a single line that swaps two variables without using a temporary. It takes a moment to read, but is this better (code wise) than using a temp?
This only works with numeric types, so I don't find it to be that useful. You've made your code less readable to someone who doesn't know the trick, which slows down maintenance of the code. I don't think it's worth the savings of one temp variable.
Upvotes: 10
Reputation: 140061
You should strive to write readable code, not code that does a lot of things in one line or has fancy tricks.
If I worked on a project with you I would likely want to strangle you after I read that for loop for the first time, that is if my head didn't explode trying to understand it first. Same goes for assignment within an assignment.
Upvotes: 15
Reputation: 20068
Um not trying to be a brick, but I think you tried to write:
public static void doAction(int i) {
System.out.println("i = " + i);
}
public static void loopValues(int i, int j) {
if (i > j) while (i >= j) doAction(i--);
else while (i <= j) doAction(i++);;
}
public static void main(String[] args) {
int start = 1, finish = 10;
loopValues(start, finish);
loopValues(finish, start);
}
Reason why you have 2 loops is efficiency and your case readability. Comparing is costly operation and you do not generally want to add extra compares to loop just to alter its normal course.
Upvotes: 6