0

i don't know how to do something specific at the last loop of this for loop:

String msg = "---Player List, Count:" + users.size() + "---" + brln;
for (int i = 0; i < users.size(); i++) {
    if((i - 1) == users.size()){
            msg += "--::" + users.get(i).name; //Do this at the last loop
            return; //returns the void
    }
    msg += "--::" + users.get(i).name + brln; // do this by default
}

Can you help me get this to work?

Erick Robertson
  • 32,125
  • 13
  • 69
  • 98
FFlaser
  • 111
  • 1
  • 2
  • 8

8 Answers8

4

Your condition is wrong: instead of (i - 1) == users.size() use (i + 1) == users.size() or i == users.size() - 1.

Basically (i - 1) == users.size() would match the element after the last (which clearly doesn't exist), i.e. for a list of size 5 you'd get (i - 1) == 5 or i == 6.

In the example above (i + 1) == users.size() and i == users.size() - 1 would resolve to (i + 1) == 5 and i == 5 - 1 which both result in i == 4, which is the last index in the list.

Edit: Btw, your loop is still quite odd. You basically seem to add a line break after each but the last element. Why don't you change it to something like this:

String msg = "---Player List, Count:" + users.size() + "---" + brln;
for (int i = 0; i < users.size(); i++) {
  if( i > 0){
    msg += brln;
  }
  msg += "--::" + users.get(i).name;
}

This would add a line break before every line except the first. Note how the condition is much easier.

Thomas
  • 87,414
  • 12
  • 119
  • 157
2

If you need last index just use if( i == users.size()-1)

In the example you provide you should use a StringBuilder for string concatenation. Then you only need to loop without asking anything and you can use for-each loop.

StringBuilder msg =new StringBuilder().append("---Player List, Count:").append(users.size()).append("---");
for (User user : users) {
   msg.append(brln)
     .append("--::").append(user.name)
     .append(brln);
}
nachokk
  • 14,363
  • 4
  • 24
  • 53
2

Change this from:

if((i - 1) == users.size()){

to:

if((i + 1) == users.size()){
Michael Ford
  • 851
  • 6
  • 9
2

What you want in effect is a string of entries separated by a delimiter, where the delimiter is a line break. Use this idiom:

String delimiter = "", result = "";
for (...loop init...) {
  result += delimiter;
  ...append one entry...
  delimiter = brln;
}

Other than that, building a large string by creating new string in each iteration is bad for performance as it is an O(n^2) operation. You should prefer a StringBuilder.

Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436
2

If you change your code a little, you can avoid a special case:

StringBuilder msg = new StringBuilder("---Player List, Count:" + users.size() 
    + "---"); // Note a lack of brln

for (int i = 0; i < users.size(); i++) {    
    msg.append(brln + "--::" + users.get(i).name);
}

return msg.toString();

Ideally you should use a StringBuilder for concatenating strings in a loop, as I've done above.

(This solution works in your case, since you had a header including a line break. See other solutions for a generic way to insert string X between every occurence but the last in a loop).

Duncan Jones
  • 67,400
  • 29
  • 193
  • 254
1

Change your condition to

i == users.size() - 1    

Why? In Java (and other languages) the first element in a list is at index 0, and the last element at N-1 (if N is the number of elements currently in the list), so users.size() - 1 is the index of the last element. For example, if there are 10 elements in users list, the last will be at index 9.

Full code:

String msg = "---Player List, Count:" + users.size() + "---" + brln;

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

    //if i equals to the last index, do your special handling of the loop
    if(i == (users.size() - 1)) {
        msg += "--::" + users.get(i).name; //Do this at the last loop
        break; 
    }

    msg += "--::" + users.get(i).name + brln; // do this by default
}
Melquiades
  • 8,496
  • 1
  • 31
  • 46
1

you are doing it wrong. Change your line:

if((i - 1) == users.size())

by this one:

if(i == (users.size()-1))
Rafa Romero
  • 2,667
  • 5
  • 25
  • 51
0

Although this might not help you right now (when you don't use Java 8), all this messy code will have an end with Java 8:

// requires Java 8
String msg = users.stream().map(User::name).collect(Collectors.joining(brln));
isnot2bad
  • 24,105
  • 2
  • 29
  • 50
  • I guess in time I'll find that more readable. But for now, I look at that and shudder a little. – Duncan Jones Jan 13 '14 at 14:41
  • Yes, maybe one needs getting used to it first. But what makes me shudder much more is the fact that such a tiny problem makes about 10 people discuss about it! So every simpler, less error prone solution is welcome, I think. ;) – isnot2bad Jan 13 '14 at 15:13