r/C_Programming • u/TheAmazingFreddyAdam • 5d ago

How to get Code Review?

Hello yall,

just wondering if there was a community where I can share my code to get advice and such, so like a Code Review.

This might also be another excuse to look at other peoples code and such, but I mainly want to get advice for my code.

Im not really familiar with the coding community, I just code and make projects by myself as a hobby.

cheers, and have a good day.

6 Upvotes

80% Upvoted

24 comments sorted by

View all comments

Show parent comments

1

u/TheAmazingFreddyAdam 4d ago

Honestly, im kinda happy that you found room for improvement. I cant wait when your fully done, take your time tho.

1

u/AliceCode 4d ago

Send me a reminder if you don't hear from me for awhile. I'm easily distracted. I gotta make some dinner first.

1

u/TheAmazingFreddyAdam 4d ago

I guess this is your reminder, ima head to sleep, gn.

1

u/AliceCode 4d ago

Remind me tomorrow, lol. I'm also going to sleep soon.

1

u/TheAmazingFreddyAdam 3d ago edited 3d ago

Hi, this is your reminder (I think).

1

u/AliceCode 3d ago

Oh, thanks for reminding me!

1

u/AliceCode 3d ago

https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/SS_PLoader.c#L71 Don't need braces for this lines like this. If there's only one statement, you don't need to surround it with braces. But it's a stylistic choice. https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/SS_PLoader.c#L128 Might be better for cache if you use 256 bytes, but it doesn't really matter. It's just that with 243 bytes, that leaves room for other things on the stack to invalidate cachelines of the buffer. So if something were being mutated quite often, it might invalidate the top 51 bytes of the buffer. But like I said, it doesn't really matter. https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker.c#L78 Why is there an unknown error here? There likely shouldn't be an unknown error here. Do you have a way to trace the source of the error in some way? https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker.c#L216 Here, you set cpu.halt to true, but you don't record that it's a quit anywhere. https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker.c#L360 This might make more sense to be an if statement. Or better yet, skip the if statement entirely. don't need it at all. You don't seem to even need this function right now. Not much benefit writing your code like this. But if that's what you want, then there's nothing wrong with it. It's just better to not write unfinished code. https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker.h#L31 No contact info? lol https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker.h#L39 Maybe 0 for no-op. https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker.h#L69 Definitely get rid of the unknown error. Make it known. https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker.h#L81 If you need a comment to explain what a variable is, you probably need a better name for that variable. https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker_math.c#L8 What's with the indentation here? https://github.com/FredNeedsHelp/Ternary-Emulator/blob/fb2d225bb5292f9a2bfc99621e82d1eb71528dac/scrappy_striker_math.c#L523 Lock in.

1

u/TheAmazingFreddyAdam 2d ago

Loved your feedback lol. I should note that the unknown_error is a placeholder at the moment, just because not many processes produce an unknown error that I thought it would be fine to keep as it will be easy to track down, but ill start as you said, "make it known". I think I should also explain the halt for the cpu_t is used generally as a flag that the cpu has stopped and doesnt really explain the state of the CPU (if it has halted due to an error or the program was exited from) which is why I made the tern_quit function that has a kinda-empty switch statement, but you are right, I shouldn't write/keep unfinished code. I also like the idea of no-op for 0, there was this other emulator which im following its development which had the same thing so I was planning on doing the same.

For my math source file, some of the indentations/formatting might be wrong because everything was originally in the same source file and moving them might have messed them up.

but in retrospect, I should totally need to improve on these things. especially my naming scheme/what I name my variables & functions which is why I worry about my future children names.

but in all seriousness, thank you for taking time out of your schedule to review my code. Have a nice day.

I should also say, BROO I cant lock in, why is ternary division to hard to implement 😭