patterncMinor
Reading and printing sudoku input
Viewed 0 times
readinginputprintingandsudoku
Problem
I have seen programs to solve sudoku puzzles but to be honest.. They were badly written.
First of all, readability of the program itself was horrible and the code was really long and confusing.
So I decided to start from scratch, using my own style to write really good solver.
What I have this far is reading a board input from the user and outputting the board representation:
Obviously it is written for Windows compatibility but I use it as for advantage to exploit some rules of commonly-accepted IO.
First of all, readability of the program itself was horrible and the code was really long and confusing.
So I decided to start from scratch, using my own style to write really good solver.
What I have this far is reading a board input from the user and outputting the board representation:
#include
#include
#include
#include
#include
#define FOREGROUND_LBLUE 9
#define COLORSET_MAIN 3
char sudoku [9][9];
int chrton (unsigned char chr)
{
if(chr >= '0' && chr <= '9') { return chr - '0'; }
else { return 0; }
}
int main()
{
char num = 0;
char x = 0, y = 0;
SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_LBLUE | BACKGROUND_BLUE);
printf("Validate numbers:\t\t\t\t\n\n");
SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), COLORSET_MAIN);
// Part 1 : validating hint numbers
while(y != 9)
{
if(kbhit() && isdigit(num = _getch()))
{
printf("%c"" ", num);
sudoku[x][y] = chrton(num);
if( ++x == 9 ) { putchar('\n'); x = 0; y++; }
if( not((x * x) % 3) and x) { putchar('\t'); }
if( not((y * y) % 3) and !x) { putchar('\n'); }
}
}
system("PAUSE");
return 1;
}Obviously it is written for Windows compatibility but I use it as for advantage to exploit some rules of commonly-accepted IO.
Solution
I have found a couple of things that could help you improve your code.
Avoid the use of global variables
I understand that it's just partial code so far, but it's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable as with
Don't use
There are two reasons not to use
Be careful with
Your
Consider using
It's not technically wrong, but you may wish to use
Consider using more idiomatic C
While your use of
Consider separating input and output
Right now, the
Consider returning
A non-zero return value is interpreted as an error condition on most operating systems. Normal program exit should return
Avoid the use of global variables
I understand that it's just partial code so far, but it's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable as with
sudoku in this code.Don't use
system("pause")There are two reasons not to use
system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. In this case, a better alternative would be getchar(). Be careful with
char and intYour
chrton function returns an int and then stores it in a char-sized location. Better would be to return char, since that's how sudoku is defined.Consider using
unsigned char for subscriptsIt's not technically wrong, but you may wish to use
unsigned char for the type of x and y. It eliminates a class of errors that can occur if the array subscript is a negative number. This is a fairly minor point, however.Consider using more idiomatic C
While your use of
and and not is not techncially wrong, because you have correctly included the iso646.h header, you should be aware that many experienced C programmers will be unfamiliar with these operator alternatives, and so if others read your code, it might be an impediment to their understanding of the code. Similarly, your unusual formatting of if statements as one-liners is likely to seem a bit strange to experienced programmers.Consider separating input and output
Right now, the
main function issues a prompt, reads in the sudoku board and then prints it out as each character is read. A more modular (and likely more maintainable) approach would separate these into separate functions.Consider returning
0 instead of 1A non-zero return value is interpreted as an error condition on most operating systems. Normal program exit should return
0 or EXIT_SUCCESS or simply omit it, because the compiler will automatically insert the equivalent of return 0; at the end of main.Context
StackExchange Code Review Q#69720, answer score: 6
Revisions (0)
No revisions yet.