# Programming Techniques

This topic is 2225 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Well recently, I figured I'd try to change the way I've been programming. I learned how to spread out data through multiple .cXX and .h files as opposed to just using a bunch of headers. My post here isn't geared towards an API or working with something really, I just want to know if the way I'm setting things up is garbage or what. Basically I've got a small SDL\OpenGL application that displays an image on the screen. I myself was simply surprised that any of it worked as usually when I try something new, everything fails.Right now, all I'm really worried about is the way I'm including header files and the way I'm handling variables a little bit differently. One thing I'm interested in is that for certain functions that need an integer or a struct I do &nameOfVariable so I don't have to return it and I can keep as many local variables as possible.

Another thing that I'm doing is making an int Game() loop, and what I do in my int main() is return Game() at the end of it. Alright, I've talked about my major concerns, I'll put up my code. I know it's quite a bit of code, I'm just asking for a skim over it and tell me if there's any major concern that pop out at you. I'm not looking for my code to be fixed, I just want to make sure that what I am doing isn't bad. Any suggestions are greatly appreciated.

Main.cpp

 #include <SDL.h> #include <SDL_audio.h> #include <SDL_opengl.h> #include "Init.h" #include "Game.h" int main(int argc, char* argv[]) { Screen screen; if (Initialize(0, screen, 0, 0, 0, SDL_HWSURFACE | SDL_HWACCEL | SDL_OPENGL | SDL_FULLSCREEN) < 0) { printf("Error: %s\n", SDL_GetError()); return -1; } return Game(screen); } 

Init.h:

 #ifndef SCREEN_H #define SCREEN_H #include "Screen.h" #endif int Initialize(int toggleCursor, Screen &screen, int Width, int Height, int BitsPerPixel, Uint32 Flags); 

Init.cpp:

 #include <SDL.h> #include <SDL_opengl.h> #include "Init.h" int Initialize(int toggleCursor, Screen &screen, int Width, int Height, int BitsPerPixel, Uint32 Flags) { if (SDL_Init(SDL_INIT_EVERYTHING) < 0) return -1; atexit(SDL_Quit); SDL_ShowCursor(toggleCursor); if (Width == 0 || Height == 0 || BitsPerPixel == 0) { screen.videoInfo = (SDL_VideoInfo*)SDL_GetVideoInfo(); if (Width != 0 && Height != 0) { screen.Width = Width; screen.Height = Height; } else { screen.Width = screen.videoInfo->current_w; screen.Height = screen.videoInfo->current_h; } if (BitsPerPixel == 0) { screen.BitsPerPixel = screen.videoInfo->vfmt->BitsPerPixel; screen.BytesPerPixel = screen.videoInfo->vfmt->BytesPerPixel; } else { screen.BitsPerPixel = BitsPerPixel; screen.BytesPerPixel = BitsPerPixel / 8; } screen.Flags = Flags; } else { screen.Width = Width; screen.Height = Height; screen.BitsPerPixel = BitsPerPixel; screen.Flags = Flags; screen.BytesPerPixel = BitsPerPixel / 8; } if (!(SDL_SetVideoMode(screen.Width, screen.Height, screen.BitsPerPixel, screen.Flags))) return -1; glMatrixMode(GL_PROJECTION); glLoadIdentity(); glOrtho(0, screen.Width, screen.Height, 0, -1, 1); glMatrixMode(GL_MODELVIEW); glLoadIdentity(); glClearColor(0, 0, 0, 0); glClear(GL_COLOR_BUFFER_BIT); return 0; } 

Game.h:

 #ifndef SCREEN_H #define SCREEN_H #include "Screen.h" #endif int Game(Screen &screen); 

Game.cpp:

 #include <SDL.h> #include <SDL_opengl.h> #include "Game.h" #include "Rendering.h" void EventHandle(int &quit, SDL_Event &event) { switch (event.type) { case SDL_QUIT: quit = 1; break; case SDL_KEYDOWN: switch (event.key.keysym.sym) { case SDLK_ESCAPE: quit = 1; break; default: break; } break; default: break; } } int Game(Screen &screen) { SDL_Event event; int quit = 0; GLuint IMAGE; IMAGE = LoadTexture("Image.jpg"); if (!IMAGE) return -1; while (!quit) { Render2D(0, 0, screen.Width, screen.Height, IMAGE); if (SDL_PollEvent(&event)) { EventHandle(quit, event); } SDL_GL_SwapBuffers(); } glDeleteTextures(1, &IMAGE); return 0; } 

Rendering.h:

 GLuint LoadTexture(const char* fileName); void Render2D(float X, float Y, float Width, float Height, GLuint texture); 

Rendering.cpp:

 #include <SDL.h> #include <SDL_image.h> #include <SDL_opengl.h> #include "Rendering.h" GLuint LoadTexture(const char* fileName) { SDL_Surface* Image = IMG_Load(fileName); Image = SDL_DisplayFormatAlpha(Image); if (!Image) return NULL; GLuint Texture; GLenum TextureFormat; switch(Image->format->BytesPerPixel) { case 3: if (Image->format->Rmask == 0x000000ff) TextureFormat = GL_RGB; else TextureFormat = GL_BGR; break; case 4: if (Image->format->Rmask == 0x000000ff) TextureFormat = GL_RGBA; else TextureFormat = GL_BGRA; break; default: return NULL; break; } glGenTextures(1, &Texture); glBindTexture(GL_TEXTURE_2D, Texture); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR); glTexImage2D(GL_TEXTURE_2D, 0, Image->format->BytesPerPixel, Image->w, Image->h, 0, TextureFormat, GL_UNSIGNED_BYTE, Image->pixels); SDL_FreeSurface(Image); return Texture; } void Render2D(float X, float Y, float Width, float Height, GLuint texture) { glEnable(GL_TEXTURE_2D); glPushMatrix(); if (texture) glBindTexture(GL_TEXTURE_2D, texture); glBegin(GL_QUADS); glTranslatef(X, Y, 0.0f); if (texture) glTexCoord2i(0, 0); glVertex3f(0.0f, 0.0f, 0.0f); if (texture) glTexCoord2i(1, 0); glVertex3f(Width, 0.0f, 0.0f); if (texture) glTexCoord2i(1, 1); glVertex3f(Width, Height, 0.0f); if (texture) glTexCoord2i(0, 1); glVertex3f(0.0f, Height, 0.0f); glEnd(); glPopMatrix(); glDisable(GL_TEXTURE_2D); } 

Screen.h:

 typedef struct Screen { unsigned int Width, Height, BitsPerPixel, BytesPerPixel; Uint32 Flags; SDL_VideoInfo* videoInfo; } Screen; 

##### Share on other sites
int Game(Screen &screen) doesn't modify screen, so you'd better pass screen as reference to const (const Screen & screen).
Also a function should be better a verb than a noun because a function does some action.
So UdateGame may be better than Game?

##### Share on other sites
I don't see anything major wrong. You are doing procedural programming, which is fine. I have only couple suggestions (nitpicking really)

- Why not change[font=monospace] [/font]void EventHandle(int &quit, SDL_Event &event) to int HandleEvent(SDL_Event &event)
- Rename Game() to RunGame()
- Try to be consistent in naming local variables (quit, IMAGE, Texture)
- Declare local variables as late as possible
- Drop the typedef from struct declaration (just struct Screen {...}; )

##### Share on other sites
Unless you mean to change a primitive type (char, short, int, long,float and their unsigned versions) it makes no sense to pass them as a reference as they are going on the stack or passed through a register of the same size as the reference.

Also if a function doesn't change it's input variables and those are passed by reference pass them as const references so the caller knows that his data won't be changed. Don't mark primitive types as const parameters for functions as it doesn't matter the function can take a copy of them and change the values all the same.
Once you move to classes make sure that methods that aren't changing the internal state of the object are marked as const as well, this allows you to call these functions on const instances of that class.

##### Share on other sites
Thanks for all of the advice, but I'm curious, why would I remove the typedef? Is that only required when using plain C?

##### Share on other sites
There are a couple of things that I've found really improve maintainability and readability (I tend towards OO and generics, but these should apply equally to Procedural):

Decompose functions. Every function should be short (2-10 lines) and every statement should be at the same level of abstraction. This will do wonders to avoid duplication and (if you name your functions well) dramatically improve readability. If you have a long function, and you're not sure how to split it up, first just chunk it into blocks, then look at what each of those blocks does. Whenever you end up with two short functions that are similar, try to make them identical and then remove one.

Maintain invariants, preconditions, and post conditions. I am not as inflexible about this as some (I generally don't assert them, etc.) but basically, if your function claims to do something, it should either do that thing or very clearly not do that thing. If you don't like to use exceptions, this can be tricky, but if you define (even just in your head) the input domain and output range and then stick to that, you will have far fewer edge cases. A simple way to do this if you don't want to deal with exceptions is to wrap primitive data types (such as the uints that opengl uses for everything) in validating structs. The struct might take a uint in the constructor, but will signal an error if that value is not valid.

If you are interested in programming style, the book Clean Code by Robert Martin is excellent, but is definitely aimed more at OO design :/

##### Share on other sites
Well right now I'm working with a procedural design. As a matter of fact I'd rather not use C++ if at all possible, right now. I've been working on ripping apart everything to see how it works myself, so I can understand it better. I don't like building off of someone else's code. I mean like if I wanted a game engine, I would rather make it myself so I know how it's supposed to work and I don't have to worry about screwing things up because a tutorial or documentation is out of date, or because I simply missed a step. It's why I'm moving closer to Assembly language and eventually creating my own OS, so I know how it works.

Eventually I'll go back to at least a semi object oriented design. I guess with all that said, I'd be more on the end of a software developer than a game developer. I can put all of the fancy math together, but I couldn't make a good 3D model or a 2D image to save my life.

Again thanks, but I ask again, why would I remove the typedef from the struct?

##### Share on other sites

Again thanks, but I ask again, why would I remove the typedef from the struct?

You can leave the typedef there.
But nowadays in C++ it's common to use "struct A" instead of "typedef struct A". The later is more C style.
There is difference of them in C, but not in C++.
http://stackoverflow.com/questions/612328/difference-between-struct-and-typedef-struct-in-c

##### Share on other sites
Ah, okay, thanks.

##### Share on other sites

There are a couple of things that I've found really improve maintainability and readability (I tend towards OO and generics, but these should apply equally to Procedural):

Decompose functions. Every function should be short (2-10 lines) and every statement should be at the same level of abstraction. This will do wonders to avoid duplication and (if you name your functions well) dramatically improve readability. If you have a long function, and you're not sure how to split it up, first just chunk it into blocks, then look at what each of those blocks does. Whenever you end up with two short functions that are similar, try to make them identical and then remove one.

Ten lines seems very small. Some of my functions have gotten smaller as I've worked on them, but the average is more than ten lines. I asked for advice about the code here a few months ago and no one said anything about the functions being too long.

1. 1
2. 2
frob
15
3. 3
Rutin
12
4. 4
5. 5

• 13
• 12
• 58
• 14
• 15
• ### Forum Statistics

• Total Topics
632123
• Total Posts
3004236

×