Alaa AbuZarifa
Alaa AbuZarifa

Reputation: 1249

String concatenation and appening are not working inside foreach

I have an array of objects, let's say Cars. There's an attribute in the object called "selected" with setters and getters.

The user can select/deselect cars form a recycleview and I update the data correctly using the adapter. Now when the user clicks a button I want to extract a string of IDs of all the selected cars.

So I did this.

 String cars_ids = "";
 StringBuilder car_builder = new StringBuilder();
        for (Car car: carList) {
             // I added this log to make sure that there are in fact selected cars. and there are!
            Log.e("==>> cicarty ",  car.isSelected() +"");
            if (car.isSelected()) {
                car_builder.append(car.getID()).append(",");
            }
        }

        if (!cars_ids.isEmpty()){
            cars_ids = car_builder.toString().substring(0, car_builder.toString().length() - 1).trim();
        }
        // cars_ids is always empty...!! 
        Log.e("=>> ", " cars_ids : " + cars_ids );

And I also tried with concatenation and it didn't work either

 String cars_ids = "";
        for (Car car: carList) {
            Log.e("==>> cicarty ",  car.isSelected() +"");
            if (car.isSelected()) {
                cars_ids.concat(car.getID() + ",");
            }
        }

        if (!cars_ids.isEmpty()){
            cars_ids = cars_ids.substring(0, cars_ids.length() - 1).trim();
        }
        // cars_ids is always empty again...!! 
        Log.e("=>> ", " cars_ids : " + cars_ids );

Upvotes: 0

Views: 557

Answers (5)

Satya
Satya

Reputation: 11

More better:

String cars_ids = "";
 StringBuilder car_builder = new StringBuilder();
        for (Car car: carList) {
             // I added this log to make sure that there are in fact selected cars. and there are!
            Log.e("==>> cicarty ",  car.isSelected() +"");
            if (car.isSelected()) {
                 cars_ids+=","+(car1.getID());
            }
        }
        Log.e("=>> ", " cars_ids : " + cars_ids.replaceFirst(",", ""));

Upvotes: 1

Eritrean
Eritrean

Reputation: 16498

Alternatively you can reduce your code to a one liner using Java 8:

String cars_ids = carList.stream().filter(Car::isSelected).map(Car::getID).collect(Collectors.joining(","));

Upvotes: 2

Andy Turner
Andy Turner

Reputation: 140299

if (!cars_ids.isEmpty()){

is always false with the StringBuilder because it is always empty: you are appending to the StringBuilder, then checking the String.

Use:

if (car_builder.length() > 0){

instead. And then

// cars_ids is always empty...!! 
Log.e("=>> ", " cars_ids : " + cars_ids );

Yep, always empty because you didn't change it. Assign car_ids = car_builder.toString() first.


cars_ids.concat(car.getID() + ",");

won't do anything useful because you're ignoring the return value. Instead:

car_ids = cars_ids.concat(car.getID() + ",");

or, more idiomatically:

car_ids += car.getID() + ",";

It's worth mentioning that in Java 8+, you could more easily use a StringJoiner to achieve the same.

Upvotes: 5

Azad
Azad

Reputation: 171

The if condition will always be false, change it to

if (cars_ids.isEmpty())

The string builder values will be assigned to the cars_ids variable

Upvotes: 0

Onur Arı
Onur Arı

Reputation: 512

Since strings are immutable , stringObject.concat does not change the string but actually returns a new string. Thus, you should use reassignment. See the following code.

 String cars_ids = "";
    for (Car car: carList) {
        Log.e("==>> cicarty ",  car.isSelected() +"");
        if (car.isSelected()) {
            cars_ids = cars_ids.concat(car.getID() + ","); // This line is changed
        }
    }

    if (!cars_ids.isEmpty()){
        cars_ids = cars_ids.substring(0, cars_ids.length() - 1).trim();
    }
    // cars_ids is always empty again...!! 
    Log.e("=>> ", " cars_ids : " + cars_ids );

Upvotes: 1

Related Questions