Anyone got some spare time?

Pages: 12
Telling the window manager (ratpoison) to close the program, does nothing.


Bjarne Stroustrup wrote:
Code that creates an object using new and then deletes it at the end of the same scope is ugly, error-prone, and inefficient.
1
2
3
4
5
6
//instead of 
Hoot2d *dh = new Hoot2d();
Source *s = new Source("mus/1.wav");
//to create objects you should simply do
Hoot2d dh;
Source s("mus/1.wav");


For the logging, you could have used clog. Instead of open-close every time, you should do it just one time.
Still, I don't see the point in those messages.

You seem to have unnecessary includes
http://www.cplusplus.com/forum/general/105860/#msg575315

Please read the warnings.

Member functions should use/modify the state of the object.
By instance `bmp::tile' is creating a temporary of `bmp' class and acts on it. There is no reason for it to be a member function.

Observe the rule of three. If you need a destructor, copy constructor o assignment operator, you need the three.

You've got a lot of memory management that could go wrong. They could be avoided or wrapped in a container (std::vector)

Error messages should be send to the error stream
Last edited on
Thanks for the feedback :D!

To be honest I don't see why ratpoison wont close the program, If I close it in Gnome, LXDE, XFCE, and KDE, it closes down properly.

As to the logging, I haven't actually used it since early testing and have been meaning to get rid of it; I'm just to lazy :P

as to the bmp::tile function bmpname.tile(...) just seems to make more sence to me than tile(&bmpname , ...);
maybe I overlook it, ┬┐where do you use this in that function?
┬┐where do you access any member variable?
line 185 glBindTexture(GL_TEXTURE_2D, this->id); but that doesn't matter, I wasn't arguing that it would be a good idea to make it not a member function. I was just saying that personally I like the feel of it being a member of bmp better than it being just a regular function
make it static then, that way is clear that you don't care about the caller object.
maybe calling as bmp::tile(...)

Also, try to observe const-correctness. If the function would not modify the state, make that explicit.
http://www.parashift.com/c++-faq-lite/const-correctness.html

By the way, you could use whitespace to group logical blocks
You know, I gave it more thought and I'll probably only be calling tile on Plane objects so I can keep the whole objectname->tile thing and make the tile function for bmp's a non member function; and making it const correct is probably a really good idea :P
Topic archived. No new replies allowed.
Pages: 12