0

I am trying to create a program that has the user click twice on the screen and a rectangle is drawn according to what was clicked.

Right now all I am trying to do is set my classes up to be able to correctly draw a rectangle manually without worrying about where the user clicks.

Eventually my program will be able to draw stuff like circles and triangles so I decided to use Polymorphism and have each shape type (i.e. Rectangle) be its own class that inherits from a class called Shapes.

I then have a class called Game which holds a Shapes object.

Shapes.h

#ifndef _shapes_h_
#define _shapes_h_
#include <vector>
#include "glut.h"

class Shapes
{
public:
    void DrawAll() const;
    void Add(Shapes * shape);
    virtual void Draw() const {}

protected:   
    std::vector<Shapes *> mShapes;
};

#endif  

Shapes.cpp

#include "Shapes.h"
#include <iostream>

void Shapes::DrawAll() const
{
    int i;
    for (i = 0; i < mShapes.size(); i++)
    {
        mShapes[i]->Draw();
    }
}

void Shapes::Add(Shapes * shape)
{
    mShapes.push_back(shape);
}

Rectangle.h

#ifndef _rectangle_h_
#define _rectangle_h_
#include "Shapes.h"

class Rectangle : public Shapes
{
public:
    Rectangle(std::vector<int> p1, std::vector<int> p2);
    void Draw() const;

private:
    std::vector<int> mP1;
    std::vector<int> mP2;
};

#endif

Rectangle.cpp

#include "Rectangle.h"
#include <iostream>

Rectangle::Rectangle(std::vector<int> p1, std::vector<int> p2)
{
    mP1 = p1;
    mP2 = p2;
}


void Rectangle::Draw() const
{
    std::cout << "Draw Me " << std::endl;
    glColor3d(0, 0, 0);

    glBegin(GL_QUADS);
    glVertex2d(mP1[0], mP1[1]);
    glVertex2d(mP2[0], mP1[1]);
    glVertex2d(mP2[0], mP2[1]);
    glVertex2d(mP1[0], mP2[1]);
    glEnd();
}

Game.h

#ifndef _game_h_
#define _game_h_

#include <vector>
#include "Shapes.h"

class Game
{
public:
    void Click(int x, int y);
    void Draw();

private:    
    Shapes mTest;
};

#endif

The first Game.cpp below is coded in a way of what I am trying to do, but it throws me the error Unhandled exception at 0x001AF742 in Shapes.exe: 0xC0000005: Access violation reading location 0x00000001. After looking into this, I've found that some __vfptr hidden pointer variable (that I think controls virtual stuff) gets set to 0 and causes memory issues.

Note, Click() only gets called when the mouse is pressed, Draw() is called anytime any type of event happens (e.i. mouse press, mouse release, key press, key release etc.) When the mouse is pressed, both events get called, but Click() gets called first.

Also, mTestis the variable that is the Shapes object.

Game.cpp: Doesn't Work

#include "Game.h"
#include "Rectangle.h"
#include <iostream>

void Game::Click(int x, int y)
{     
    std::vector<int> p1;
    p1.push_back(200);
    p1.push_back(200);

    std::vector<int> p2;
    p2.push_back(250);
    p2.push_back(250);

    Rectangle rect(rp1, rp2);

    Shapes * rectangle = &rect;
    mTest.Add(rectangle);
}

void Game::Draw()
{       
    mTest.DrawAll();
}

However, what seems to baffle me is if I add a Shape Rectangle to mTest inside of the Draw() function right before I call DrawAll(), it works. However, I want to be able to create the Rectangle depending on where the User clicks, and this method wont allow that.

Game.cpp: Works

#include "Game.h"
#include "Rectangle.h"
#include <iostream>

void Game::Click(int x, int y)
{     
}

void Game::Draw()
{
    std::vector<int> p1;
    p1.push_back(200);
    p1.push_back(200);

    std::vector<int> p2;
    p2.push_back(250);
    p2.push_back(250);

    Rectangle rect(rp1, rp2);

    Shapes * rectangle = &rect;
    mTest.Add(rectangle);
    mTest.DrawAll();
}
tysonsmiths
  • 485
  • 1
  • 8
  • 19
  • 2
    You have a dangling pointer in the first example, as soon as `Game::Click` returns `rect` no longer exists. – user657267 Apr 11 '15 at 22:48
  • So what your saying is that `rect` gets destroyed when `Game::Click` ends, so when I added `rectangle` to `mTest`, it ends up pointing to nothing when the function ends. How could I get around this? – tysonsmiths Apr 11 '15 at 22:52
  • Make `mShapes` a `std::vector>`, this will make sure the class properly handles the lifetime of the shapes while still allowing them to be used polymorphically. – user657267 Apr 11 '15 at 23:02
  • I've never heard of `unique_ptr` but I'll give it a try. This however means that I need to go through all my code an modify it to work with `uniqure_ptr`. This is fine except at the point where I do `Shapes * rectangle = &rect;` When I try to do `std::unique_ptr rectangle = &rect;` I get errors. How do I handle this expression when using the `unique_ptr`? – tysonsmiths Apr 11 '15 at 23:09
  • To be honest your design is a bit wonky, why would shapes be both a shape and a container? If you absolutely want to stick with this then one quick and dirty fix would be to replace `mTest.Add(rectangle);` with `mTest.Add(new Rectangle(rp1, rp2));` but this might cause stuff to leak if and when exceptions come into play. – user657267 Apr 11 '15 at 23:15
  • I was thinking that it was weird using it as a container and a shape. I'm going to try holding the vector of different shapes inside the `Game` class. – tysonsmiths Apr 11 '15 at 23:18

1 Answers1

1

In Shapes class replace

std::vector<Shapes *> mShapes; 

by

std::vector<std::shared_pointer<Shapes>> mShapes; 

Than replace code

Rectangle rect(rp1, rp2);

Shapes * rectangle = &rect;
mTest.Add(rectangle);

by

mTest.Add(std::make_shared<Rectangle>(rp1, rp2));

And finally replace method definition

void Shapes::Add(Shapes * shape)

by

void Shapes::Add(std::shared_ptr<Shapes> shape)

It makes your code works.

Summary: If you need pointers, think about smart pointers (std::unique_ptr mentioned by @user657267 is smart pointer too). You can read about smart pointers in many places, here and here. For better design, do not forget for design patterns.

Community
  • 1
  • 1
gomons
  • 1,946
  • 14
  • 25