Code Review Request: Sudoku GUI built with Win32 API in C

[star_8232]

Background:

I am a beginner programmer and I wrote a Sudoku GUI 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.

Code : https://github.com/reewdgh/sudoku_gui

Concerns

Please guide me on:

• Bugs or potential issues
• Efficiency
• Naming anything I could simplify or improve
• How can i move to to tier 1 tier 2

I'd appreciate your feedback on my code.

[rproffitt]

If it works, move on to the next project.

[rproffitt]

Looked over the code. What needs more efficiency?

As to the next level, that's your next level in programming.

[vztwonaim]

Hi! I’d be happy to review your Sudoku GUI code. Since you’re still learning, don’t worry too much about efficiency at this stage. Please share your code, and I can provide constructive feedback on the structure, Win32 API usage, readability, and possible improvements.

[rproffitt]

Please guide me on:

Bugs or potential issues
Efficiency
Naming anything I could simplify or improve

Looks fine from here. I downloaded the .zip file and read it. Now you need to work on that next level. I'll just say it, I won't be writing it for you. This is your app, keep it yours.

[star_8232]

Looked over the code. What needs more efficiency?

As to the next level, that's your next level in programming.
if I would look over the code and knew what could be more effeicent then why would i come here?

[star_8232]

Please guide me on:

Bugs or potential issues
Efficiency
Naming anything I could simplify or improve

Looks fine from here. I downloaded the .zip file and read it. Now you need to work on that next level. I'll just say it, I won't be writing it for you. This is your app, keep it yours.

Listen, in my eyes, I don't think anything is wrong, but that's not the point. What if there are mistakes, inconsistencies etc. I don't know. How am I supposed to learn from my mistakes and if I coded a new app would I make the same mistakes while not know how I am supposed to move from tier 1 to tier 2?

[Reverend Jim]

Just so you know, I think you have 30 minutes from the time you post something to edit it for any changes/corrections. You don't have to delete it and post it again.

[rproffitt]

Try this. Write the app for tier two and get it working. Then merge that to your first app and call that new procedure when needed.

[trcooke]

I'm not exactly sure what you mean by tier 1 and tier 2, but I assume you mean a measure of programming proficiency? I had a quick scan over your source code repo and the main thing that jumps out at me is that you have no automated tests that verify the behaviour of your code. Granted, writing automated tests for UI elements is notoriously difficult, but the essence of Sudoku is mathematical so there should be plenty of opportunity to write some tests that verify your code game logic is good.

Have a look for some unit test frameworks for the C language and see if you can use it to write some automated tests.

In a professional setting, I like to say that delivering code without tests is like offering your boss and teammates a pinky promise that your code behaves as required. It doesn't feel very satisfactory, does it.

[danny_32]

ngl this is pretty solid for a beginner winapi project in C. like actually getting a working gui is a flex. for bugs i'd double check memory leaks especially when generating new puzzles and closing the window cuz C can be messy like that. also test what happens if the generator gives an unsolvable puzzle might crash.for efficiency you're redrawing the whole grid every time when you could just update the changed cells but honestly for sudoku it doesnt matter that much.naming wise some functions are way too long like sudoku_generate_puzzle_with_difficulty just call it gen_puzzle or something shorter. also that giant drawing function could be split into smaller ones for readability.tier 1 stuff add undo and redo timer save load and highlight errors. tier 2 make it customizable with themes and keyboard input and maybe a hint system. if you really wanna flex try porting to direct2d for smoother rendering.keep grinding man good work fr

[star_8232]

Try this. Write the app for tier two and get it working. Then merge that to your first app and call that new procedure when needed.

My question is that if i have to transfrom this app in to tier 2 app what can i do ?

[rproffitt]

I wouldn't "transform" the app. I'd write a new app that played the 2nd tier game. Then you'll have to code to merge and a better understanding of the language.

[star_8232]

Hi! I’d be happy to review your Sudoku GUI code. Since you’re still learning, don’t worry too much about efficiency at this stage. Please share your code, and I can provide constructive feedback on the structure, Win32 API usage, readability, and possible improvements.

Hello @vztwonaim, Did you forget ??

[Salem]

I see you've taken some of my suggestions, but made some other things worse.
https://forums.codeguru.com/showthread.php?566573-CODE-REVIEW-Code-Review-Sudoku-GUI-in-C-language

First off, the way you use git is to NOT delete and re-upload every single time you have something to show.

You made the naming worse here.

20,23d17
<     int pos1;
<     int pos2;
<     int x;
<     int y;
24a19
>     int positions[4];

Great, I change positions[2], what happens? How is this obvious to me?

struct PointerStruct is similarly badly named.

Maybe struct Game renames to struct Sudoku, then struct PointerStruct renames to struct Game

Your handling of isStart in three separate places as three special cases needs to be fixed. A separate init function that is called once from just one place should be all you need.

If you want something 'tier 2' to do, then handle WM_SIZE properly.
https://learn.microsoft.com/en-us/windows/win32/winmsg/wm-size

Rectangle(hdc, val, val, val 9.65, val 4 - 9);

You're going to have to think a bit as to how you calculate all these given only the size of the client area that Windows tells you.

[vztwoaeee]

Nice project for a beginner! Since the program is already working, I’d focus first on readability and maintainability rather than optimizing everything prematurely. You could look for repeated logic that can be moved into functions, use clearer variable/function names, and separate the Sudoku-solving logic from the Win32 GUI code where possible.

[Contesthelper44]

Nice project for a beginner! Try separating the Sudoku logic from the GUI code. Focus on clean naming, input validation, and simple unit tests. That will make your code easier to maintain and improve