Critique
Moderator: Coders of Rage
Critique
I have written a very small pong example, I didn't add score support. But could you please take a look and critique my design and programming style. Thanks. Note that it's in C not C++. http://projects.dejai.org/devel/pong/src/main.c
- dandymcgee
- ES Beta Backer
- Posts: 4709
- Joined: Tue Apr 29, 2008 3:24 pm
- Current Project: https://github.com/dbechrd/RicoTech
- Favorite Gaming Platforms: NES, Sega Genesis, PS2, PC
- Programming Language of Choice: C
- Location: San Francisco
- Contact:
Re: Critique
Looks very neat and easy to read. Other than that dunno what to say about Pong code.
Falco Girgis wrote:It is imperative that I can broadcast my narcissistic commit strings to the Twitter! Tweet Tweet, bitches!
Re: Critique
I'm not an C expert, but I guess you have a memory leak in your initGame function. You are returning by value so why don't you use a stack variable?
Re: Critique
I don't understand your question. Perhaps there is one but the initGame just returns Game which is a struct game*, an instance of Game called newGame is created in the stack and set to malloc(sizeof(struct game)); which is on the heap. Parameters are set and it is returned destroying the pointer from the stack, we use this pointer to define our true instance of Game pong = initGame(); (since it's the return we don't worry about it) We then free it of course with closeGame(pong), are you saying that newGame the pointer to struct game is a memory leak?
Re: Critique
My method has the same effect as returning; return malloc(sizeof(struct game));
Re: Critique
Oops, I did not see this:
However, that is really confusing. Use game* if you are returning a pointer. You are saving only a single character so it's really not worth the confusion.typedef struct game* Game;
- Falco Girgis
- Elysian Shadows Team
- Posts: 10294
- Joined: Thu May 20, 2004 2:04 pm
- Current Project: Elysian Shadows
- Favorite Gaming Platforms: Dreamcast, SNES, NES
- Programming Language of Choice: C/++
- Location: Studio Vorbis, AL
- Contact:
Re: Critique
I figured that i would throw my 2 cents in on this as well, since I'm a fairly experienced C programmer.
You're using C, not C++, so there really isn't so much about your "design" that we can critique (consider the fact that "good OO design" is the number one thing people bitch about on these forums). From a C programming perspective, your code is beautiful. Very nicely organized, very neat, very easy to follow. I don't know how much experience you have, but most C programs wind up looking considerably less organized.
Now about this subject:I mean honestly, what have you really achieved there other than saving a character and confusing experienced programmers? Kbal knows what he's talking about, and he thought you had a memory leak just because of that typedef. At first glance, I agreed with him. Programmers need to be able to see what is a pointer and what isn't.
The only typedef that I really approve of isBecause you're only avoiding retyping "struct" every time, which is something that programmers are used to in C++.
You're using C, not C++, so there really isn't so much about your "design" that we can critique (consider the fact that "good OO design" is the number one thing people bitch about on these forums). From a C programming perspective, your code is beautiful. Very nicely organized, very neat, very easy to follow. I don't know how much experience you have, but most C programs wind up looking considerably less organized.
Now about this subject:
You definitely don't have a memory leak, but I don't think you should be doing:Kbal wrote:I'm not an C expert, but I guess you have a memory leak in your initGame function. You are returning by value so why don't you use a stack variable?
Code: Select all
typedef struct game* Game;
The only typedef that I really approve of is
Code: Select all
typedef struct {
} name;
Re: Critique
GyroVorbis wrote:
The only typedef that I really approve of isBecause you're only avoiding retyping "struct" every time, which is something that programmers are used to in C++.Code: Select all
typedef struct { } name;
there are many text that show it this way
Code: Select all
typedef struct {
} name, *nameP;
Some person, "I have a black belt in karate"
Dad, "Yea well I have a fan belt in street fighting"
Dad, "Yea well I have a fan belt in street fighting"
Re: Critique
Thanks for the comments and criticisms. The typedef struct game* Game is a borderline call I will admit probably just typedefing the struct game to game then using a pointer would have been more appropriate and readable, I will do that in the future. I will implement that in my next project, "tetris". In terms of C experience I have just done a first semester course at uni in C and read two or three good books on it.
I have also been influenced by one of my Tutors who guided me through how to write respectable code and I use a similar style guide to that of Google (some what modified). I have little experience doing graphical programming, I have done some 3d work with Irrlicht but I have never got my hands dirty with SDL and OpenGL and that's what I am trying to do now. I was reading an article on Game Dev that talked about becoming a proficient games programmer and I also remember Gyro talking about developing a portfolio of work. So I am just going through all the classic games starting of course with Pong, then going Tetris, then Space Invaders, Pacman.. Then into some more complicated projects such as mario and a fighter plane side scroller which I will develop a small engine for and probably make a map editor. Kind of a exponential in terms of effort for reward, but by the end of mario I should be pretty proficient in openGL and SDL.
Would you mind if I posted my code for each project as I finish them to get criticism? After I finish tetris I will be moving to C++ since the games almost need it to maintain a neat structure. I have worked with game states before in C++ but I might ask for some advice on design when the time comes.
On ugly C, have you ever seen any GNU applications, yuk. e.g "printf(_("String")); #define _(x) gettext(x)
I have also been influenced by one of my Tutors who guided me through how to write respectable code and I use a similar style guide to that of Google (some what modified). I have little experience doing graphical programming, I have done some 3d work with Irrlicht but I have never got my hands dirty with SDL and OpenGL and that's what I am trying to do now. I was reading an article on Game Dev that talked about becoming a proficient games programmer and I also remember Gyro talking about developing a portfolio of work. So I am just going through all the classic games starting of course with Pong, then going Tetris, then Space Invaders, Pacman.. Then into some more complicated projects such as mario and a fighter plane side scroller which I will develop a small engine for and probably make a map editor. Kind of a exponential in terms of effort for reward, but by the end of mario I should be pretty proficient in openGL and SDL.
Would you mind if I posted my code for each project as I finish them to get criticism? After I finish tetris I will be moving to C++ since the games almost need it to maintain a neat structure. I have worked with game states before in C++ but I might ask for some advice on design when the time comes.
On ugly C, have you ever seen any GNU applications, yuk. e.g "printf(_("String")); #define _(x) gettext(x)
Re: Critique
How do I make games?dejai wrote:[..] So I am just going through all the classic games [..]
Re: Critique
Ya that's what I said.
Re: Critique
It wasn't a question, just reminded me of that document, but I see now that it might look like one :P
Re: Critique
That page reminds me that I havn't done some projects yet I will now make a Pong in the next hours :D