Traversing List of Integers and ending on the last index which should be 0 I have a method that checks if a List is solvable. Solvable in this context means 2 things:


The last index of the list is a 0.
Starting from the first index, each value indicates how many position you should go back or forth in the List. After walking the indexes you should end at the last index.




public class ListSolver {

  public boolean isSolvable(List&lt;Integer&gt; list) {
    if (list == null) {
      throw new IllegalArgumentException("List is null");
    }

    int listSize = list.size();
    int lastIndex = listSize - 1;
    int currentIndexValue;
    for (int i = 0, loopCounter = 0; i &lt; listSize; i = i + currentIndexValue, loopCounter++) {
      try {
        currentIndexValue = list.get(i);
      } catch (IndexOutOfBoundsException e) {
        return false;
      }

      // if traversed to last index and last index is 0
      if (i == lastIndex &amp;&amp; currentIndexValue == 0) {
        return true;
      }

      // if infinite loop
      if (loopCounter &gt; listSize) {
        return false;
      }
    }

    return false;
  }

}




public class ListSolverTest {

  private ListSolver listSolver = new ListSolver();

  @Test(expected = IllegalArgumentException.class)
  public void isSolvableGivenNullExpectIllegalArgumentException() {
    listSolver.isSolvable(null);
  }

  @Test
  public void isSolvableGivenListContainingZeroAndZeroExpectFalse() {
    boolean isSolvable = listSolver.isSolvable(Arrays.asList(0, 0));

    assertFalse(isSolvable);
  }

  @Test
  public void isSolvableGivenListContainingOneAndNegOneExpectFalse() {
    boolean isSolvable = listSolver.isSolvable(Arrays.asList(1, -1));

    assertFalse(isSolvable);
  }

  @Test
  public void isSolvableGivenListContainingZeroExpectTrue() {
    boolean isSolvable = listSolver.isSolvable(Collections.singletonList(0));

    assertTrue(isSolvable);
  }

  @Test
  public void isSolvableGivenListContainingOneAndZeroExpectTrue() {
    boolean isSolvable = listSolver.isSolvable(Arrays.asList(1, 0));

    assertTrue(isSolvable);
  }

  @Test
  public void isSolvableGivenListContainingNegOneExpectFalse() {
    boolean isSolvable = listSolver.isSolvable(Collections.singletonList(-1));

    assertFalse(isSolvable);
  }

}


The method was written using TDD, but I'm stuck now.


  How  can I refactor and make it more readable without creating new methods that feel contrived?
This function relies on no object state, so I would make it static.

You don't need both listSize and lastIndex.  To reduce clutter, I would keep just lastIndex.

currentIndexValue is too cumbersome a name, in my opinion.  "Current" is superfluous, especially when there is no "previous" or "next" involved.  "Index" just adds confusion.  So, I'd simply call it value.

The for loop is trying to do too much.  Considering that currentIndexValue is defined elsewhere, you can't understand how the loop behaves just by looking at the loop header.  So, I would just make it count iterations.

If currentIndexValue is 0, but i is not lastIndex, then i is never going to move.  You might as well return false immediately in that case.

It would be better to avoid triggering IndexOutOfBoundsException when i is negative.

public static boolean isSolvable(List&lt;Integer&gt; list) {
    if (list == null) {
        throw new IllegalArgumentException("List is null");
    }

    int lastIndex = list.size() - 1;
    int i = 0;
    for (int loopCounter = 1; loopCounter &lt;= lastIndex; loopCounter++) {
        if (i &lt; 0 || i &gt; lastIndex) {
            return false;
        }
        int value = list.get(i);
        if (value == 0) {
            return i == lastIndex;
        }
        i += value;
    }

    return false;    
}