patterncsharpMinor
Sudoku Valid Arangements Permutations Enumerator
Viewed 0 times
arangementsvalidenumeratorsudokupermutations
Problem
I wondered if anyone has any ideas to make this cleaner or more efficient. Mostly this was an exercise to generate the data once (So speed really isn't too important, but I'd love to see what techniques you could think of)
This is an algorithm I derived and implemented without external guidance of any kind, so I'm asking for guidance now.
This does give the output I am expecting, I validated it with a recursive implementation that I made first.
I'm thinking this might be helpful in validating solutions, or in generating new boards.
For the Sudoku table:
1 would be represented by the array: 238316751. In the first 9 cell box, the 1 is at position 2 in the array of that box.
```
///This outputs the valid pointers
///for example 327742830 is valid, a number could be in pos 3 of cell 0, 2 in cell 1, etc
/// So arrays are compatable when no columns have the same number in them
/// A starting board with a high number of compatable arrays would be very difficult i believe
public IEnumerable OutputArrayPoints()
{
return OutputArrayPoints(new int[9]);
}
//Will contain the cell indexes that are not excluded by other cells already
int[][] ToCheck = new int[9][];
//Pointers to the current cell indexes in use
int[] ToCheckPointers = new int[9];
//arrayPoints will contain the current value to return
//Really it should be equal to loop through ToCheck[c][ToCheckPointers[c]]
public IEnumerable OutputArrayPoints(int[] arrayPoints)
{
//This loop is basically a depth first search
//ToCheck gets filled with possible values for each cell,
// and the lowest pointer gets incremented until it runs out,
// then moves up a cell to go down a different path
for (int c = 0; c >= 0; /++c/)
{
if (c == 8 && ToCheck[c] != null && ToCheckPointers[c] CellsToCheck(int[] arrayPoints, int depth)
{
int rowSkip1 = (depth % 3) > 0 ? arra
This is an algorithm I derived and implemented without external guidance of any kind, so I'm asking for guidance now.
This does give the output I am expecting, I validated it with a recursive implementation that I made first.
I'm thinking this might be helpful in validating solutions, or in generating new boards.
For the Sudoku table:
--1 --- ---
--- 1-- ---
--- --- --1
--- -1- ---
1-- --- ---
--- --- 1--
--- --- -1-
--- --1 ---
-1- --- ---1 would be represented by the array: 238316751. In the first 9 cell box, the 1 is at position 2 in the array of that box.
```
///This outputs the valid pointers
///for example 327742830 is valid, a number could be in pos 3 of cell 0, 2 in cell 1, etc
/// So arrays are compatable when no columns have the same number in them
/// A starting board with a high number of compatable arrays would be very difficult i believe
public IEnumerable OutputArrayPoints()
{
return OutputArrayPoints(new int[9]);
}
//Will contain the cell indexes that are not excluded by other cells already
int[][] ToCheck = new int[9][];
//Pointers to the current cell indexes in use
int[] ToCheckPointers = new int[9];
//arrayPoints will contain the current value to return
//Really it should be equal to loop through ToCheck[c][ToCheckPointers[c]]
public IEnumerable OutputArrayPoints(int[] arrayPoints)
{
//This loop is basically a depth first search
//ToCheck gets filled with possible values for each cell,
// and the lowest pointer gets incremented until it runs out,
// then moves up a cell to go down a different path
for (int c = 0; c >= 0; /++c/)
{
if (c == 8 && ToCheck[c] != null && ToCheckPointers[c] CellsToCheck(int[] arrayPoints, int depth)
{
int rowSkip1 = (depth % 3) > 0 ? arra
Solution
-
You have some "Magic Numbers". If you wanted to work with a different grid size you'd have to replace
-
I like that you are using
-
Your code is quite hard to understand, even with your comments. This can have something to do with your variable names probably.
-
Instead of using integer arrays, you could use
-
-
As you are not incrementing the
-
Slightly related is my Code Review question for a Sudoku Solver, it might give you an idea of how things can be organized into a more object-oriented approach
-
All things considered, I must say that your code seems to be quite fast. I am quite sure it can be made faster but it's hard to tell how exactly because of the lack of clarity of the code.
You have some "Magic Numbers". If you wanted to work with a different grid size you'd have to replace
9 at many different places. Find the relationships between your hard-coded numbers and declare them as const. For example, const int Size = 9; and const int Box = 3;-
I like that you are using
yield-
Your code is quite hard to understand, even with your comments. This can have something to do with your variable names probably.
ToCheck for example, if that is checking rows, columns, values, what kind of values, etc. is not clear at all from the name. From what I understand, you are sometimes considering the array indexes as index of a 3x3 grid, and sometimes as a row/column. That's confusing for me.-
Instead of using integer arrays, you could use
Set.-
int[][] ToCheck and int[] ToCheckPointers should be local variables inside the OutputArrayPoints method, they do not seem to be used outside it.-
As you are not incrementing the
c variable on every iteration, I would use a while (c >= 0) rather than for.-
Slightly related is my Code Review question for a Sudoku Solver, it might give you an idea of how things can be organized into a more object-oriented approach
-
All things considered, I must say that your code seems to be quite fast. I am quite sure it can be made faster but it's hard to tell how exactly because of the lack of clarity of the code.
Context
StackExchange Code Review Q#20888, answer score: 6
Revisions (0)
No revisions yet.