FFlaser
FFlaser

Reputation: 121

How do I get this for loop to work right?

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?

Upvotes: 0

Views: 152

Answers (8)

Melquiades
Melquiades

Reputation: 8598

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
}

Upvotes: 1

Duncan Jones
Duncan Jones

Reputation: 69399

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).

Upvotes: 2

nachokk
nachokk

Reputation: 14413

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);
}

Upvotes: 2

Thomas
Thomas

Reputation: 88757

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.

Upvotes: 4

isnot2bad
isnot2bad

Reputation: 24464

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));

Upvotes: 0

Marko Topolnik
Marko Topolnik

Reputation: 200296

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.

Upvotes: 2

Rafa Romero
Rafa Romero

Reputation: 2716

you are doing it wrong. Change your line:

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

by this one:

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

Upvotes: 1

Michael Ford
Michael Ford

Reputation: 861

Change this from:

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

to:

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

Upvotes: 2

Related Questions