Counting linear conflicts of the state of 8 puzzle for heuristic

I need to find linear conflicts of 8 puzzle state, state is represented by int[9], goal state is {1,2,3,4,5,6,7,8,0}. A linear conflict would be if in a line two tiles that are supposed to be in that line are reversed.

For example, in goal state, the first row is 1,2,3 if in the state the first row is 2,1,3 then that is one linear conflict made by tiles 2 and 1.

My code works, but is way too long and awkward. Here it is:

```
public static int linearConflicts(int[] state) {
ArrayList row1 = new ArrayList();
ArrayList row2 = new ArrayList();
ArrayList row3 = new ArrayList();
ArrayList column1 = new ArrayList();
ArrayList column2 = new ArrayList();
ArrayList column3 = new ArrayList();
int[] columnMarkers = new int[] { 0, 3, 6, 1, 4, 7, 2, 5, 8 };
for (int i = 0; i rowState) {
int conflicts = 0;
if (rowState.contains(1)) {
if ((rowState.contains(2))
&& rowState.indexOf(1) > rowState.indexOf(2)) {
conflicts++;
}
if ((rowState.contains(3))
&& rowState.indexOf(1) > rowState.indexOf(3)) {
conflicts++;
}

}
if (rowState.contains(2) && rowState.contains(3)
&& rowState.indexOf(2) > rowState.indexOf(3))
conflicts++;
return conflicts;
}

public static int row2Conflicts(ArrayList rowState) {
int conflicts = 0;
if (rowState.contains(4)) {
if ((rowState.contains(5))
&& rowState.indexOf(4) > rowState.indexOf(5)) {
conflicts++;
}
if ((rowState.contains(6))
&& rowState.indexOf(4) > rowState.indexOf(6)) {
conflicts++;
}

}
if (rowState.contains(5) && rowState.contains(6)
&& rowState.indexOf(5) > rowState.indexOf(6))
conflicts++;
return conflicts;
}

public static int row3Conflicts(ArrayList rowState) {
int conflicts = 0;
if (rowState.contains(7) && rowState.contains(8)
&& rowS

I would have some tweaks how you could improve your code quality. Reading it from the top I would start adding a comment for your method linearConflict and mention that the array as the argument contains two dimensional matrix mapped into a 1D array. It is a short comment and then immediately you get the idea why the next variables are called columns and rows.

Another thing is I would avoid using i < 9 in for (i = 0; i < 9; i++). I know it is supposed to be 9 but in case it is not then you will get ArrayOutOfBound. I would say better would be to use i < state.lentgh or i < CONSTANT. The CONSTANT would be defined as static final in the class, but then I would suggest checking the length of the state array so that you know it will not be shorter - just more defensive style of programming.

The methods row1conflict, row2conflict,.... look exactly the same just the numbers inside the method are different. I would suggest have those numbers as the arguments and then you can have only one method. I would also make the numbers as constants in the class because the particular numbers does not change and then you can pass the constants in the method. This prevents you from running into a situation when you find out that you make a mistake in one method and since all the method are "the same" you have to correct it in multiple places.

So the signature could look:

public static int arrayConflicts(List array, int number1, int number2,....)
//And then call it like this
arrayConflict(column1, NUMBER_1, NUMBER_2, NUMBER_3....)

I named the arguments number1 and number2 just for illustration but if you can come up with more descriptive name that would be better.

Another point as a type of variables use List instead of ArrayList. It gives you the freedom to change later on ArrayList for something else without refactoring all the types of the variables.

StackExchange Code Review Q#44427, answer score: 5