0

What I'm trying to do with this code is that it gives the user a choice between 2 items in a list and the selected item is taken to a new list in the list of lists. I just wanted to know if the implementation of my new list creation is correct or no. I have another non related bug in the program so I can't test if the implementation works or not.

public static void compareTwo(List<List<String>> list){
       int result;
        for (int j = 0;j<list.size();j++) {
            if ((list.get(j).size() / 2) % 2 == 0) {
                for (int i = 0; i < list.get(j).size(); i = +2) {
                    String x = list.get(j).get(i);// gets stuff inside the inner list
                    String y = list.get(j).get(i+1);
                    System.out.println("1 ("+x+") or 2 ("+y+")" );
                    result = Integer.valueOf(scan.nextLine());

                    list.add(j + 1, new ArrayList<String>() {
                    });// add another inner list to biglist

                    list.get(j+1).clear();//clears the new inner list

                    if (result==1){
                        String z = list.get(j).get(i);
                        list.get(j+1).add(z);
                    }
                    else{
                        String z = list.get(j).get(i+1);
                        list.get(j+1).add(z);
                    }


                }
    }
Motcho
  • 25
  • 1
  • 5

3 Answers3

1
list.add(j + 1, new ArrayList<String>() {
});// add another inner list to biglist

list.get(j+1).clear();//clears the new inner list

This is creating an anonymous subclass of ArrayList: unless you've got a good reason to do that, don't. Also, you don't need to clear a newly-created ArrayList, it's already empty.

As such, you can reduce this code to:

list.add(j + 1, new ArrayList<>());
Andy Turner
  • 137,514
  • 11
  • 162
  • 243
  • Okay thanks. I'll do that. Can you explain what "This is creating an anonymous subclass of ArrayList" means? I'm kinda new to this programming thing – Motcho Apr 23 '20 at 19:19
  • 1
    https://docs.oracle.com/javase/tutorial/java/javaOO/anonymousclasses.html – Andy Turner Apr 23 '20 at 19:38
1

I can see two serious problems in your code:

  1. Since the following line is trying to access value at index i + 1, the for loop must be for (int i = 0; i < list.get(j).size() - 1; i = +2) to avoid IndexOutOfBoundsException. In your code it is, for (int i = 0; i < list.get(j).size(); i = +2).
String y = list.get(j).get(i+1);
  1. In the following line, you are not handling exceptions for the cases when the user enters a non-integer value.
result = Integer.valueOf(scan.nextLine());

It should be like

boolean valid;
do {
    valid = true;
    try {
        result = Integer.parseInt(scan.nextLine());
    } catch (NumberFormatException e) {
        System.out.println("This is not an integer. Please try again.");
        valid = false;
    }
} while (!valid);

Apart from these two serious problems, the second line in the following code snippet doesn't make sense as there is no element to clear in the newly added inner list

list.add(j + 1, new ArrayList<String>() {});// add another inner list to biglist    
list.get(j+1).clear();//clears the new inner list
Arvind Kumar Avinash
  • 71,965
  • 6
  • 74
  • 110
0

You can use this version for simplicity and readability

List<String> inner = new ArrayList<>();
list.add(j + 1, inner);

, or some java's double-brace initialization

list.add(j + 1, new ArrayList<>(){{ 
  add("hello"); 
  add("world");
}});
0xh3xa
  • 4,801
  • 2
  • 14
  • 28
  • 1
    "java's hidden feature" This isn't a hidden feature, it's a syntactic abuse called [double-brace initialization](https://stackoverflow.com/questions/1958636/what-is-double-brace-initialization-in-java), and should be approached with great care. – Andy Turner Apr 23 '20 at 19:46