Welcome to Code Forum!

Join a community that supports you and your coding journey from day one. We strive to be a friendly, supportive community that empowers everyone to be better developers. By registering with us, you'll be able to discuss, share and private message with other members of our community.

SignUp Now!
  • Guest, before posting your code please take these rules into consideration:
    • It is required to use our BBCode feature to display your code. While within the editor click < / > or >_ and place your code within the BB Code prompt. This helps others with finding a solution by making it easier to read and easier to copy.
    • You can also use markdown to share your code. When using markdown your code will be automatically converted to BBCode. For help with markdown check out the markdown guide.
    • Don't share a wall of code. All we want is the problem area, the code related to your issue.

    GIF shows where to locate </> in the thread and or post editor toolbar.
    To learn more about how to use our BBCode feature, review our "How to post your code into threads" here.

    Thank you, Code Forum.

Code Review : Sudoku GUI in C language

user_232

user_232

New Coder
Background:

I am a beginner programmer and I wrote a Sudoku GUI in using winapi32 in C language.

It is currently working and does what it is supposed to do, but because I am still learning, I know it is likely inefficient and could be written much better.

GitHub repo link: GitHub - reewdgh/gui_sudoku

Please guide me on:
  • Bugs
  • Efficiency
  • Naming anything I could simplify or improve
  • inconsistency
I'd appreciate your feedback on my code.
 
Sort by date Sort by votes
My first impression of generateValidSudoku was: that's a lot of recursion (81 levels?), and it appears to be doing a backtracking search, that would take forever.

To my surprise it finds a solution very quickly, sudoku grids are more common than I first thought.

A non recursive version of generateValidSudoku would be about the same length, run in the same time, and be easier on the eye.
 
Upvote 0 Downvote
BlackCube said:
My first impression of generateValidSudoku was: that's a lot of recursion (81 levels?), and it appears to be doing a backtracking search, that would take forever.

To my surprise it finds a solution very quickly, sudoku grids are more common than I first thought.

A non recursive version of generateValidSudoku would be about the same length, run in the same time, and be easier on the eye.
Click to expand...
Thanks for your review, but yeah, it works as the standard way to generate a sudoku and it is very easy for me, so can you share a little about how I can generate a sudoku without backtracking/recursion?
 
Upvote 0 Downvote
Nothing wrong with backtracking. What I meant was, doing a backtrack with a simple loop and a few states.

Put it this way, a function that computes a factorial could use a loop, or it could use one recursive call at the end. Both are looping, but only one is easy to read, and the other is using the stack for no good reason.

On the other hand, I have no problem with your approach, its actually kind of interesting. A non recursive solution would need a 81 by 9 array for the random numbers. Your recursion uses a 1 by 9 and the stack.
 
Upvote 0 Downvote
Play Snayk 3+, higher dimensional snake game?
I would code a function called check, which calls checkRows and checkCols and checkSubGrid. That would make the IF much tidier.

The FOR loop that shuffles the numbers 1 to 9 is the kind of thing that can be made into a seperate function. A function that shuffles any array would be useful in other programs as well.

One other thing, this is my own private hobby-horse, and many disagree with me. I hate it when a function has more than one return statement. I would rather chew my own hand off than code like that. All my functions have one return, and its always at the bottom. But hey, that's just me.
 
Upvote 0 Downvote
Thanks all of you that, I honestly didn't see those things that you have pointed out to me.
Sorry to call this out, but I really suck at pointers and structures etc. I have really been staring at the code for hours and I don't even know how to solve some specific problems. It takes a lot of time for me to solve them. Could you recommend some advice on how I could fix this, or could you recommend me some C codebase that I should read to improve my coding skills etc. Whatever helps?
 
Upvote 0 Downvote
Code: 
    int numbers[9] = {1, 2, 3, 4, 5, 6, 7, 8, 9};

    for (int i = 0; i < 9; i++)
    {
        int random = rand() % 9;
        int temp = numbers[i];
        numbers[i] = numbers[random];
        numbers[random] = temp;
    }
I'm going to be pedantic. There is a subtle problem here. If I pick a number, like 3, I would expect (in a fair random shuffle) the probability of 3 ending up in the first position to be the same as the last position.

I haven't done the math yet, but a quick monte carlo simulation tells me the probability is not 1/9.

Always choosing the random element forwards, removes the bias.
Code: 
FOR I=0 TO 8-1
  R = random from I to 8
  SWAP position I and R
 
Upvote 0 Downvote

New Threads

Latest posts

Share this page

Buy us a coffee!

Buy me a coffee.
DataCamp Advertisement
Code Academy Advertisement

About Us

Code Forum is a community platform where coding enthusiasts can connect with other developers, engage in discussions, ask for help, and share their knowledge with a supportive community. It's a perfect place to improve your coding skills and to find a community of like-minded individuals who share your passion for coding.

Site links

Stay Connected

Back
Top Bottom