Simplify a loop [closed]

Question

Closed. This question is off-topic. It is not currently accepting answers.

---

Questions must involve real code that you own or maintain. Questions seeking an explanation of someone else's code are off-topic. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example.

Closed 10 years ago.

Locked. This question and its answers are locked because the question is off-topic but has historical significance. It is not currently accepting new answers or interactions.

What would your preferred loop type be for a case like this:

StringBuilder sb = new StringBuilder();
for(Integer id : idList) {
 sb.append("?,");
}

In short: In dependency to the size of a List i want to add stuff to a String. The above example produces a "unused variable" warning.

I'm thinking of these alternatives:

for(int i=0; i<idList.size(); i++) {
 // ...
}

I don't really like all the extra typing there...

Iterator it = idList.iterator();
while(it.hasNext()) {
 // ...
 it.next();
}

The loop head itself looks nice, but i end up typing a lot of extra code (getting the iterator, moving to the next element...)

What I'd like to do would be something like that:

for(idList) {
 // ...
}

Is there a better style/What is the best kind of style for that kind of loop?

Answer 1

I would use a "normal" string repeating method:

String s = repeat("?,", idList.size());

Unfortunately Java doesn't have such a method in the standard API. So you could write one yourself:

String repeat(String s, count i) {
 StringBuilder sb = new StringBuilder();
 while (i-- > 0) {
 ab.append(s);
 }
 return sb.toString();
}

or use one provided by several "standard add-on APIs" such as Apache Commons' StringUtils.repeat().

• Steven Jeuris: +1 That is a good alternative (or perhaps better) solution than using closures indeed! :) I was thinking way too technical hehe. :)
• E-K: I don't dislike the answer, but I think the construct "while(i-- >0) is not as clear as a more often seen for(int i=0; i<count; i++).. This is more intuitive IMO.

Answer 2

The second example is by far the clearest in expressing your intention. You want to do something an amount of times, in this case, idList.size().

for( int i = 0; i < idList.size(); ++i ) {
 // ...
}

In the third one you are basically just writing what the first example already generates, so that's definitely not an improvement.

You do have a point that a for loop is rather a big code construct to express just executing a piece of code an amount of times. Ruby's times operator is made exactly for this purpose. In other languages there are some code constructs available which could help you. Extension methods in C# allow you to create a similar operator as the times operator of Ruby.

For Java I don't believe a more concise syntax is possible since closures aren't supported in Java by default.

---

As in RoToRa's answer, a simple String helper class containing a Repeat function is most likely the cleanest solution for you. It doesn't provide the functionality to repeat any type or operation, but it does provide the concise syntax you are looking for.

• Michael K: I've heard noises that Java 7 will support closures :)
• Steven Jeuris: @Michael K: Java will have to if they don't want to lose programmers to other languages. ;p

Answer 3

I suggest to use Strings.repeat() from Google Guava library:

String result = Strings.repeat("?,", idList.size());

Answer 4

I would go with the first alternative and use @SuppressWarnings.

If you need this pattern more often, how about an object oriented solution:

public static class Repeat<T> implements Iterable<T> {
 private final T[] ts;
 private final int n;
 public Repeat(int n, T ... ts) {
 this.ts = ts;
 this.n = n;
 }
 public Iterator<T> iterator() {
 return new Iterator<T>() {
 int count = 0; 
 public boolean hasNext() {
 return count < n;
 }
 public T next() {
 if (! hasNext()) {
 throw new NoSuchElementException();
 }
 return ts[(count++) % ts.length];
 }
 public void remove() {
 throw new UnsupportedOperationException("Not supported yet.");
 }
 };
 }
 public static <U> Repeat<U> repeat(int n, U ... us) {
 return new Repeat<U>(n, us);
 }
}
//usage
import static blabla.Repeat.*;
...
for(String s : repeat(idList.size(), "?,")) {
 sb.append(s);
}

• Steven Jeuris: So where exactly does this improve on the given example? I don't see any advantage (or extra reuseability) in using this class.
• RoToRa: It's a nice OO approach (+1), but far too verbose for the presented problem (-1). Thus no vote in either direction from me :-)
• Steven Jeuris: @Landei: My downvote is not because this is a bad solution in its own right, but because its a bad solution for the posed problem. Now I know you shouldn't worry about performance until required, but neglecting it altogether for no added conciseness/clarity is worth a downvote in my personal opinion. Why pass the string around through iterators if its a const? If other people feel different they can still upvote. Don't take it so personal.
• Landei: @Steven Jeuris: There is always a trade-off between reuseability and performance. I think it makes sense to look for the most flexible and general solution for a given problem, and having an iterator that gives back a fixed list of things is certainly useful. I think the performance hit isn't as bad as you think, and you don't seem to bother much about performance yourself when suggesting closures, which would be exactly as "bad" as an Iterable performance-wise. And again, your solution with a loop var and range checks and inc doesn't look nice to me. But I would never downvote it.
• asveikau: This "simplifies" for(int i=0; i<foo.size(); ++i) how? I'm all for the "don't repeat yourself" maxim but this is pushing it. The best part is "Not implemented yet" - are you planning to implement removal in the future?

Answer 5

I'd personally use the second version, i.e. the for loop because it is clearer. The first example looks like you've introduced an error by not using the id, the third adds two extra lines of code, but isn't too bad on the eyes.

I think I would also create a local cache of the size() so that you can avoid the function call on every iteration of the loop. But this is a compromise between speed, the size of the list, readability and whether the list size will change during the loop.

int size = idList.size();
for (int i = 0; i < size; ++i) {...}

If this is a heavily called inner loop the saving yourself a few tens of thousands of function calls to return back a "constant" can save some time. But if you do something like this it might be worth a comment to say why you did it "differently" to an normal expected way.

I like for loops because they are very self contained; you get start value, end condition and increment operation all on one line whereas a while loop requires hunting for the "++" somewhere within the while block itself.

I think keeping your intention simple and clearly seen is more important going forward when your code has to be maintained. Remember to code as if the person maintaining your code is serial axe murderer who know where you live!

• Steven Jeuris: While I totally agree with your answer, it doesn't add much more to the given ones. Consider expanding it a bit.