The results of trySim are too broad

[ronniecross]

I should consider using trySim to return a collection, and then perform various checks on them. For example, as I am parsing a coordinate, I could get a list of all possible numbers in that coordinate. Then, if the container is not a square, I can check all coordinates in the same container that share a square to see what is possible.

[ronniecross]

Perhaps instead of returning this in a collection and doing this in the already sprawling solve() method (and potentially doubling the workload), I should consider having a helper method in group container that trySim() uses before returning a number.

[ronniecross]

I added the following to trySim which seems to have caused an infinite loop:

} else { // Get a list of possible numbers in this coordinate. If there // is only one, assign it to the result. Set<Number> possibleNumbers = coord .getPossibleNumbers(getRemainingNumbers(allNumbers)); if(possibleNumbers.size() == 1) { Iterator<Number> i = possibleNumbers.iterator(); result = i.next(); } else { // If there are more than 1 results, for each possible number, // parse the other coordinates in this non-square container // that share the same square to Number, to see if any number // can only go into this coordinate. If a number is found, // assign it to result. // try row first: Set<Coordinate> rowCoords = coord.square.getBlankCoordinates(coord.row); Set<Number> rowNums = coord.row.getRemainingNumbers(allNumbers); for(Number n : rowNums) { if(coord.possible(n)) { if(n.getPossibleCoords(rowCoords).size() == 1) { result = n; } } } if(result == null) { Set<Coordinate> colCoords = coord.square.getBlankCoordinates(coord.column); Set<Number> colNums = coord.column.getRemainingNumbers(allNumbers); for(Number n : colNums) { if(coord.possible(n)) { if(n.getPossibleCoords(colCoords).size() == 1) { result = n; } } } } } }

[ronniecross]

^^^ well, I was expecting formatting 👎

[ronniecross]

I commented the following, which stopped the infinite loop:

Set<Coordinate> rowCoords = coord.square.getBlankCoordinates(coord.row); Set<Number> rowNums = coord.row.getRemainingNumbers(allNumbers); for(Number n : rowNums) { if(coord.possible(n)) { if(n.getPossibleCoords(rowCoords).size() == 1) { result = n; } } } if(result == null) { Set<Coordinate> colCoords = coord.square.getBlankCoordinates(coord.column); Set<Number> colNums = coord.column.getRemainingNumbers(allNumbers); for(Number n : colNums) { if(coord.possible(n)) { if(n.getPossibleCoords(colCoords).size() == 1) { result = n; } } } }

[ronniecross]

This infinite loop is weird.

1. I put a break point on if(result == null) and that hit, so I assumed nothing before that was the problem. So,
2. I commented out if(result == null) and everthing in the scope of the if statement, and got an infinite loop.
3. I wondered if it was a case of every time this method returns a number, an infinite loop is generated, but that doesn't seem to be the case.

[ronniecross]

Update on the infinite loop (I should really start considering creating a new issue for this): I've adapted the code on puzzle: `if(..) { // Some Code

    if(..) {
            trySimResult = false; // BreakPoint
        }
    }
}
if (!trySimResult) {
    failedIteration = true; // BreakPoint
}`

I put a break point on the two marked rows. The first marked row sets TrySimResult to false, while the second marked row is accessed if TrySimResult is false. While the first row is hit, the second isn't.

[ronniecross]

The infinite loop is now fixed, although getByShortlist seems to be producing weird results. To confirm the strange results were coming from getByShortlist, I identified an erroneous coordinate, then set a flag to only trigger if we're working with that coordinate, then set a breakpoint after every opportunity to set the result. The only breakpoint to be hit was the one immediately below 'getByShortlist()'

NEXT: I want to investigate getByShortlist in the same way (set a debug flag for when this coordinate is hit) and then see what is happening every step through getByShortlist.

[ronniecross]

I think I know what's happening!

When a number is found, it's added to the result set, and the result set is then returned by the method, rather than coords being updated. When getByShortlist is run, it checks the current coordinates, however does not take into consideration coordinates that have already been changed.