Java ternary operator and setting loop index values

I have a for loop looping through an ArrayList.

If a condition is met in the for loop:

• I remove the current element from the ArrayList
• reduce the size of the ArrayList local variable
• reduce the for loop's index like so, checking to ensure that they never go below zero.

A case where we have just removed the last element of the ArrayList:

i = (i > 0) ? i-- : i;

My problem is that the above does not reduce i by 1 when i > 0. I've used ternary operators countless times, but never seen this behavior. I've tested that i is indeed > 0 and that the i-- section is being called. It simply isn't reducing the value of i. Removing the 0 value check and simply running i--; does indeed reduce i as expected.开发者_如何学C

EDIT2: Ok, well someone edited out my last edit where I mentioned that I am specifically NOT using a ListIterator in this case due to the performance sensitive nature of the loop itself, being in a critical portion of Android code.

i-- decrements i, but returns the original value.

i = i-- will decrement i, then assign it to its original value.

You should use i - 1.

Smallest fix:

You may be after something like this:

for (int i = 0; i < l.size(); i++) {         <--------------------------------.
    if (cond) {                                                               |
        l.remove(i);                                                          |
        i--;              // even if i == -1, it will be set back to 0 here --'
    }
}

Backward iteration instead:

Another common solution is to iterate backwards, like this:

for (int i = l.size() - 1; i >= 0; i--) {
    if (cond) {
        l.remove(i);
    }
}

Using ListIterator (unless highly performance critical):

However, you're even better off with a ListIterator:

Iterator<String> iter = l.iterator();
while (iter.hasNext()) {
    if (shouldRemove(iter.next())
        iter.remove();
}

Did you try?:

i = (i > 0) ? --i : i;

With pre-decrement you should solve all your problems.

Having an assignment such as i = i-- (which happens for i > 0) simply doesn’t make sense: what exactly are you expecting here? Use the following instead:

i = (i > 0) ? i - 1 : i;

Or, even better:

if (i > 0)
    i--;

Do the right thing: use an iterator to loop over your array list and safely remove items while iterating.

The algorithm you have described is a bad strategy for what you are trying to do: removing an element from an ArrayList is an O(n) operation so applying this to the entire ArrayList is O(n^2) in the general case.

A much better strategy:

• Create a new empty ArrayList
• Loop once through the original ArrayList, and add only the elements you want to keep to the new ArrayList

This is O(n), and will actually be simpler code (you don't have to worry about fiddling with the loop bounds etc.)

Consider not using a ternary operator with a noop.

if (i > 0) { --i }

exactly describes the functionality you want (based on your example).

As others have said, i-- decrements i and returns the original value, meaning that your code would have performed the decrement, but then immediately set it back to its original value.

You could switch it around to be --i, in which case the decremented value will be returned, but either way you're doing un-necessary work with the --, because it has to set i, return the new value, and then set i again to the new value.

You'd be better off just doing i-1, which won't set the value of i twice:

i = (i > 0) ? i-1 : i;

All that said, since the false option of the ternary doesn't do anything, you'd probably be better of just using a straightforward if():

if(i > 0) { i--; }

It's easier to read, doesn't matter which way round the -- goes, and doesn't do any unnecessary processing, whatever the result.