how to fix iterating through this arraylist?

Question

I'm using two buttons to iterate back and forth in an arraylist that contains 12 months, when i reach the last element and press the prevButton the app crashes, this also happens when i go back to the first element and hit the nextButton, how do i fix my if statment?

I have tried to do it with >= and <= but it didn't work

nextButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {

                if (i < months.size()) {
                    Month month = months.get(i);
                    monthTextView.setText("" + month);
                    i++;
                }
            }
        });


        prevButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {

                if (i >= 0) {
                    Month month = months.get(i);
                    monthTextView.setText("" + month);
                    i--;
                }
            }

        });

the main problem is that you are getting the value before changing i - so to say, i is pointing to the next value - do as 'm.antkowicz' wrote and increment/decrement before getting the month - and a little note: adding month to an empty string is a bit weird, use ...setText(month.toString()) — user85421
– user85421, Commented Aug 11, 2019 at 19:43

m.antkowicz · Accepted Answer · 2019-08-11 19:46:55Z

1

with the last click you are changing the i value anyway to -1 or months.size() by adding/subtracting 1 after accessing last/first element

let us take a look closer on this fragment:

if (i >= 0) { // let us assume that i = 0, we are getting inside
    Month month = months.get(i); // you are assigning first element of months, everything is fine
    monthTextView.setText("" + month); // some data operation - irrelevant
    i--; // anyway you are updating i to [i = -1]
}

// one "next" click later...

if (i < months.size()) { // condition is fine because i = -1
    Month month = months.get(i); // oooops tried to access months[-1] - ERROR
    monthTextView.setText("" + month);
    i++;
}

The same is happening in the second scenario

What you should do - you should update i value only if current value is strongly > 0 or < month.size() - 1

if (i >= 0) {
    Month month = months.get(i);
    monthTextView.setText("" + month);
    i = (i > 0) ? (i - 1) : 0; // for example like this
}

edited Aug 11, 2019 at 19:46

answered Aug 11, 2019 at 19:40

m.antkowicz

13.7k22 silver badges43 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Michael Over a year ago

I would suggest it is cleaner to always update i, but as the first operation within the if statement, and change the bound checks. Otherwise the view will take two taps to update correctly

m.antkowicz Over a year ago

basically you are right, however the question is how OP wants his 'list' to be handled after accessing the last/first element. Should it repeatedly show first/last item, do nothing, go to the last/first element like in carousel etc

user85421 Over a year ago

agreed with @Michael - it is like if the index is already zero (prev button), do nothing; or already at end (next), do nothing; and it also means that i is always pointing to the actual value

Michael Over a year ago

With the current implementation, when you first click next/prev then nothing will happen, assuming that the initial value of i used to populate the text view (as we will repopulate with the data for the current value of i, and then update i).

user85421 Over a year ago

thinking of it, strange behavior using that code - e.g. i == 0; press next and see January; press next and see February; press prev and see March ??!! and no matter if next or prev we'll get February

user · Accepted Answer · 2019-08-11 19:50:57Z

1

when you reach the last or first index the final value will be -1 or maxsize of it but when you click the opposite button back the condition is still pass, for example when you already at -1 the next button condition still pass since -1 is < than month.size() your condition should be something like if i-1 <= 0 and then set i

Edit: I think the person who posted this means that when ( i == months.size() - 1 ) and you say next button, it works fine right then because it gets the last month but then i increases by one, that is, i == months.size() is true now. Then, when you press the prevButton, it is only checked if i >= 0 and not i < months.size(). Thus, the array index is out of bounds now. You should check the index bounds in both button methods.

edited Aug 11, 2019 at 19:50

user

7,6243 gold badges18 silver badges47 bronze badges

answered Aug 11, 2019 at 19:44

Potato

9181 gold badge9 silver badges19 bronze badges

1 Comment

Potato Over a year ago

if i - 1 == 0 then he just do his own logic and then set i-- the same logic still apply he still see the first and the last month

Collectives™ on Stack Overflow

how to fix iterating through this arraylist?

2 Answers 2

5 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related