Some thoughts on the code

Some thoughts on the code

Anonymous

Hullo,

I found FLARE today, and have so far been enjoying it greatly.  I took a look at the code (wasn't sure how to change keybindings).  In general, I think the code is good (I have little experience with SDL, to be honest), but I'd thought I'd mention the following things:

  • GameState is used polymorphically, but it does not contain a virtual destructor.  In particular, GameSwitcher deletes currentState, which is a GameState* and sometimes points to derived classes.  This is, if I am not mistaken, undefined behaviour.
  • Classes are written in accordance with RAII and delete the pointers they own; however, why not take things a step further and make them scoped_ptrs, unique_ptrs or shared_ptrs as appropriate?  Pointers that need to have SDL_Free called on them can be wrapped similarly -- this is how Wesnoth handles the situtaion (http://wiki.wesnoth.org/CodingStandards#Write_Exception-Safe_Code).
  • Several headers have `using namespace std;'.  While this isn't an issue if the code is only used internally by the project, this means that anyone including those headers also has the entire std namespace imported, which may not be appreciated.
  • A number of places seem to need eight directions, and are written as case statements:  this is fine, but a global array of (-1, 1) through (0, 1) would make more sense to me.

I also noticed that the Linux tarball seems to contain a bunch of hidden files (probably created by some backup utility?). Apologies if this sounds overly negative, I do love the game and am looking forward to future version. -jesyspa