-----------------------------------

abcdefghijklmnopqrstuvwxy

Mon Jan 22, 2007 10:31 pm


Code Critique

-----------------------------------

I hope no one is code shy...  Please critique my code as harshly as possible.  

Output:  mastermind.cpp.  (The driver program)  The output is the board, complete with one tac of each color drawn to the default tac location.  
 
mastermind.cpp

#include "SDL_MainWindow.hpp"
#include "MasterMind.hpp"


void initialize_tacs(std::vector*, Board&);
void display_tacs(std::vector*, SDL_Surface*);

int main(int args, char** argv)
{
  try
  {
    SDL_Window window(SCREEN_WIDTH, SCREEN_HEIGHT, BPP, 
                    SDL_HWSURFACE | SDL_DOUBLEBUF, "MasterMind");
    Board board;
  
    std::vector* pTacs = new std::vector;
    pTacs->reserve(board.size());
    initialize_tacs(pTacs, board);
    bool quit = false;
    while (quit == false)
    {
      SDL_Event event;
      while (SDL_PollEvent(&event)) 
      {  
        if (event.type == SDL_QUIT)
          quit = true;
	if (event.type == SDL_MOUSEBUTTONDOWN)
        {

        }
      }
      display_tacs(pTacs, board.me());
      board.drawOn(window.me());      
      SDL_Flip(window.me());
    }
  }
  catch(...)
  {
    cerr begin()); i != pTacs->end(); ++i)
  {
    i->drawOn(board);
  } 
  return;
}




"MasterMind.hpp"

#include 
#include 
#include 
#include 

enum Color {BLACK, WHITE, RED, BLUE, YELLOW, PURPLE, GREEN, BROWN, ORANGE};

const std::string black="black_tac.gif", white="white_tac.gif", 
                  red="red_tac.gif", blue="blue_tac.gif", 
                  yellow="yellow_tac.gif", purple="purple_tac.gif", 
                  green="green_tac.gif", brown="brown_tac.gif";

const std::string boardFile = "main_board.jpg", cordFile = "holes.cfg", tacFile = "tac.cfg";

const int SCREEN_WIDTH = 800, SCREEN_HEIGHT = 900, BPP = 32;

const int RADIUS = 18,  //the radius from a point which is the center of a hole on the board.
          TAC_RADIUS = 30;
using std::cerr;
using std::endl;

SDL_Surface* loadImage(std::string filename);

class Point
{
 public:
  int x, y;

  Point(int x_cord = 0, int y_cord = 0): x(x_cord), y(y_cord)
  {}
  ~Point()
  {}
  Point& operator=(Point const &other_cord) 
  { 
    this->x = other_cord.x;
    this->y = other_cord.y;
    return *this;
  }
};

class Hole
{
 private:
  Point center;
  bool status;
  int ID;
 public:
  Hole();
  Hole(Point& cord, int id): center(cord), ID(id)
  {}
  ~Hole()
  {}
  void setPointandID(Point cord, int id) { center = cord;  ID = id; }
  void setSatusOccupied() { status = true; } 
  bool isOccupied() const { return status; }
  bool isOnHole(const Point&) const;
  int getID() const { return ID; }
  Point getCenter() const { return center; }
};

//my implementation of a has-a relationship, where board has-a Point...

class Board
{
 private:
  SDL_Surface* board;
  const int columns;
  const int rows;
  std::vector guess_holes;
  std::vector answer_holes;
 public:
  Board();
    ~Board();
  Hole& findNearestHole() const; //need a sort algo for that...
  void drawOn(SDL_Surface*);
  SDL_Surface* me() const { return board; }
  int size() const { return columns*rows; }
  
  //public data
  std::vector tacDefaults;
};

/* vector ::iterator vIter; standard way to iterate over a vector and 
insert values...

   for ( vIter = vec.begin ( ) ; vIter != vec.end ( ); vIter++)
*/

class Tac
{
 private:
  Point location;
  SDL_Surface* tac;
  Color color;
 public: 
  Tac(Color, Board&);
  SDL_Surface* getSurface() const { return tac; }
  void drawOn(SDL_Surface*);
};  



"MasterMind.cpp"

#include 
#include 
#include 
#include 
#include 
#include "MasterMind.hpp"



SDL_Surface* loadImage(std::string filename)
{  
   SDL_Surface* loadedImage = NULL;   
   SDL_Surface* optimizedImage = NULL;   
   loadedImage = IMG_Load(filename.c_str());  
   if (loadedImage != NULL) 
   {
     optimizedImage = SDL_DisplayFormat(loadedImage); //to ensure format is same for image and surface to be blitted. 
     SDL_FreeSurface(loadedImage);  
   } 
   else
     cerr > x;
    guess_holeS >> y; 
    Point cordinate(x,y);
    Hole hole(cordinate, num);   //set cordinate and ID on hole
    guess_holes.push_back(hole); //then add the hole to the vec.    
  } 
  while (!tac_defaults.eof())
  {
    tac_defaults >> num; //num is aka color...
    tac_defaults >> x;
    tac_defaults >> y;
    Point cordinate(x,y);
    Hole hole(cordinate, num);
    tacDefaults.push_back(hole);
  }
}  

Board::~Board()
{
  SDL_FreeSurface(board);
  
}

void Board::drawOn(SDL_Surface* dest)
{
  SDL_Rect location;
  location.x = 0;
  location.y = 0;
  SDL_BlitSurface(board, NULL, dest, &location);
}

/*
** End Board
*/



"SDL_MainWindow.hpp"

#include 
#include 
#include 


class SDL_Window
{
  private:
    SDL_Surface* pMainWindow;
    const int  _BPP_;
    const int SCRHEIGHT;
    const int SCRWIDTH;
    const int INIT_OPTIONS;
  public:
  SDL_Window(int,int,int,int,std::string);
    ~SDL_Window();
    SDL_Surface* me() const { return pMainWindow; }
};



"SDL_MainWindow.cpp"


#include 
#include 
#include "SDL_MainWindow.hpp"
#include 

using std::cerr;
using std::endl;

SDL_Window::SDL_Window(int sw, int sh, int bpp, int options, std::string):_BPP_(bpp), SCRWIDTH(sw), SCRHEIGHT(sh), INIT_OPTIONS(options)
{
  if (SDL_Init(SDL_INIT_AUDIO|SDL_INIT_VIDEO) < 0)
  {
    cerr push_back(tac);
  }
  return;
} 

Why are you bothering with the variable "tac", and why the return statement?  Let's see what this could look like.

void initialize_tacs(std::vector* pTacs, Board& board)
{
  for (int i = 0; i < 8 ; i++)
  {
    pTacs->push_back(Tac((Color)i, board));
  }
} 


-----------------------------------

abcdefghijklmnopqrstuvwxy

Tue Jan 23, 2007 11:57 pm


RE:Code Critique

-----------------------------------

Cool, thanks.  I didn't think of that.  Question though, is it better to create a std::vector or a std::vector*.  Someone mentioned a performance increase with the former, but I cannot see how.


-----------------------------------

wtd

Wed Jan 24, 2007 12:57 am


RE:Code Critique

-----------------------------------

They are utterly different types.


-----------------------------------

abcdefghijklmnopqrstuvwxy

Wed Jan 24, 2007 1:06 am


RE:Code Critique

-----------------------------------

Yes and I'm asking which is better.   I could use either one to make my program work.

So there is no confusion: I know that std::vector is a vector of pointers to Tac objects and std::vector* is a pointer to a vector which contains Tac objects.

:D


-----------------------------------

wtd

Wed Jan 24, 2007 1:49 am


RE:Code Critique

-----------------------------------

I guess it just seems like apples and oranges to me.  Choose whichever more clearly solves the problem.


-----------------------------------

abcdefghijklmnopqrstuvwxy

Wed Jan 24, 2007 4:15 am


Re: Code Critique

-----------------------------------

Here's an archive with my project.  Currently there is a segmentation bug that I cannot locate.

To compile just type make.  But you need SDL installed and you also need SDL_image which is an extension.