Is there a better way to search for arbitrary values ​​in this list?

Question

I need to do this, check the import of java.sql.PreparedStatement, I have a parsing tree that contains the import instructions in this list, and I want to check it, the alredy code works, but it doesn’t look the best it might be is the best way to check this list?

List<DetailAST> packageDefinition = findAllAstsOfType(aAST, TokenTypes.IDENT); for (int j = 0; j < packageDefinition.size() - 2; j++) { if (packageDefinition.get(j).getText().equals("java")) { if (packageDefinition.get(j + 1).getText().equals("sql")) { if (packageDefinition.get(j + 2).getText().equals("PreparedStatement")) { importsPreparedStatement = true; } } } } 

Answer 1

Besides combining your 3 ifs into one:

 if(statement1 && statement2 && statement3) 

There are other ideas that come to mind:

If the value of importsPreparedStatement should be true only once, you can break; after setting it to true . Therefore, if it makes no sense to search more after you set the field to true , use break; .

From a design point of view, you can combine an if into a method, for example doesImportPreparedStatement or isImportingPreparedStatement , or possibly containsImportPreparedStatement .

I see one more thing. You repeat your packageDefinition , but you check the three elements one after another. I assume they are grouped in groups of 3 so that you can do something like this:

 for (int j = 0; j < packageDefinition.size() - 2; j += 3) 

From a design point of view, if I were you, I would put these 3 elements in my own class, in which case you can simplify things, and it will look like this:

 for(DefinitionElement e : packageDefinitions) { if(e.doesImportPreparedStatement()) { importsPreparedStatement = true; break; } } 

In the latter case, the DefinitionElement type will contain three elements that you group together in your array, and a method that can determine whether it contains the import of ready-made statements or not. I think this form is more readable and easy to maintain. From my experience, indexing is not fun, and you have to understand the context in order to know what j + 2 means.

If you do not want (or cannot) move them to your class, you can at least give a name to the index j + 2 and j + 1 , in order to find out later what they mean.

Answer 2

I do not really understand your DetailAST class and how you inserted differenct objects into the list, but at least you can use && instead of a nested if statement.

 List<DetailAST> packageDefinition = findAllAstsOfType(aAST, TokenTypes.IDENT); for (int j = 0; j < packageDefinition.size() - 2; j++) { if (packageDefinition.get(j).getText().equals("java") && packageDefinition.get(j + 1).getText().equals("sql") && packageDefinition.get(j + 2).getText().equals("PreparedStatement")) { importsPreparedStatement = true; break; } } 

Answer 3

Use the && operator to make it one of the conditions. For instance:

 if ((packageDefinition.get(j).getText().equals("java")) && (packageDefinition.get(j + 1).getText().equals("sql")) && (packageDefinition.get(j + 2).getText().equals("PreparedStatement"))) { importsPreparedStatement = true; } 

Since the Java && operator makes a short circuit .

For example, when (packageDefinition.get(j).getText().equals("java")) evaluates to false. The other two will not be evaluated simply because it would be optional.

Answer 4

You can try this solution, which will find the array inside the larger array:

 public static int findArray(Integer[] array, Integer[] subArray) { if (Collections.indexOfSubList(Arrays.asList(array), Arrays.asList(subArray)) != null) { importsPreparedStatement = true; { } 

Just do subArray[] = {"java,"sql",PreparedStatement"};