-4

The method is supposed to revers the order of items in the List

public class ZrcalnaSlikaSeznama {
    public static <T> List<T> zrcalnaSlika(List<T> seznam) {
        int length = seznam.size();
        List<T> seznam2 = new ArrayList<>();
        for (T x : seznam){
            seznam2.add(length-1, x);
            length--;
        }
        return seznam2;
    }
}

But it throws IndexOutOfBoundsException. Why? And how to fix the method?

Zabuzard
  • 25,064
  • 8
  • 58
  • 82
Miha Ljubi
  • 9
  • 1
  • 3
    What is the value of `length-1` when the ArrayList is empty? – sleepToken Jan 24 '20 at 18:18
  • 3
    Change your strategy: iterate on the original list backwards, and add to the new list. To know why you get the exception, read the javadoc of add(). – JB Nizet Jan 24 '20 at 18:19
  • 1
    Use Collections.reverse() to reverse the list – Mike Reddington Jan 24 '20 at 18:30
  • 1
    This is what you're looking for : https://stackoverflow.com/questions/3962766/how-to-get-a-reversed-list-view-on-a-list-in-java – dashuser Jan 24 '20 at 18:33
  • 4
    Downvoted because you did not provide any information about your actual issue. Your code is missing the error information, the stacktrace, an explanation and the example input that actually leads to the crash. Please read [ask] and [mcve], thanks. – Zabuzard Jan 24 '20 at 18:37
  • The naming of your `length` variable is bad. It starts at length but you decrement it, suggesting that the list shrinks, but its not. Actually, your `length` is not the length but an `index` of the next item you want to touch. Consider renaming it. – Zabuzard Jan 24 '20 at 18:43

2 Answers2

1

Explanation

The main issue of your code is that you assumed you could directly add something to a later position in an ArrayList, using add(index, element).

But it is not an array, it is a list. This is not possible. The add method does not allow adding beyond its current size, which is still 0. As explained in the documentation of this method:

Throws: IndexOutOfBoundsException - if the index is out of range (index < 0 || index > size())

Fix

To fix your code, and still going this approach, you would first have to ensure that the list already has the desired size by prefilling it with garbage data, like lots of nulls. And then doing what you tried to do, but by using set, not add (otherwise you would not change existing entries but shift them around). So something like this:

List<T> reversed = new ArrayList<>();

// Fill with garbage
for (int i = 0; i < seznam.size(); i++) {
    reversed.add(null);
}

// Exchange against elements
int i = seznam.size() - 1;
for (T element : seznam) {
    reversed.set(i, element);
    i--;
}

Proper solution

Obviously, filling the list first with garbage data is not an ideal approach. You can do much better. How about adding to your new list forwards, but iterating the original items backwards. So if you have [1, 2, 3] you add, 3, 2 and finally 1. This can be accomplished easily:

List<T> reversed = new ArrayList<>();

for (int i = seznam.size() - 1; i >= 0; i--) {
    reversed.add(seznam.get(i));
}

Note

There are also other approaches to this and Java also has already built-in methods for this, if that is an option for you, i.e. Collections.reverse(list).

For example, see How to get a reversed list view on a list in Java?

Zabuzard
  • 25,064
  • 8
  • 58
  • 82
0

Another alternative to reverse the order of your items in the List using the for-loop() could be :

public class ZrcalnaSlikaSeznama {

    public static <T> List<T> zrcalnaSlika(List<T> seznam) {
        int length = seznam.size();
        List<T> seznam2 = new ArrayList<>();

        for (int i = length-1; i>=0; i--){
            seznam2.add(seznam.get(i));
        }

        return seznam2;
    }

}
that_hijabi_programmer
  • 1
  • 1
  • While this works, it does not explain why OPs code failed. Which is likely the reason why OP asked in the first place. – Zabuzard Jan 24 '20 at 19:00