2

I'm relatively new to C++ and have been trying to troubleshoot this problem for awhile now, but can't figure this out. I have 2 classes, one called Polygon and one called Rectangle. Rectangle is a child class of Polygon. Polygon contains a std::vector<Points> called _points.

I would like my Rectangle constructor to add some Points to _points based on the constructor arguments. That sounded simple enough. However, when I add Points to _points, they seem to get lost outside of the constructor. When I check the size of _points inside the constructor, it gives the right value. However, when I check it back in the main program, right after constructing the Rectangle, it is 0. I think there is something going on under the hood I don't know about. Any ideas on what could be wrong?

Here's some snippets of code I think may help
Polygon.cpp

// This is just for reference, there are a few other classes involved
// but I don't think they should effect this.
Polygon::Polygon() 
    : Object()
    , _lastColor(0,0,0)
    , _lastDotColor(.5, .5, .5)
{
    _points = vector<Point>();
    _colors = vector<RGB>();
    _dotColors = vector<RGB>();
    _numDotSections = -1;
}  

Rectangle.cpp

Rectangle::Rectangle(Point p1, Point p2, RGB lineColor, double dotSeg, RGB dotColor) : Polygon(){
    _points.push_back(p1);
    // Prints out 1
    std::cout << "Point size: " << getPoints() << std::endl;
}

Driver.cpp

Rectangle playerPolygon = Rectangle(Point(1,1,-.75), Point(-1,-1,-.75), RGB(1,1,1));
// Prints 0
cout << "Number of points (Rectangle): " << playerPolygon.getPoints() << endl;

Edit1: Per request, here's the majority of the text for Polygon.cpp

// File imports
#include <GL/glut.h>
#include <iostream>
#include <math.h>

#include "Polygon.h"

using namespace std;

/**
 * Default constructor for Polygon.  Uses initializer lists
 * to setup the different attributes
 */
Polygon::Polygon() 
    :  _lastColor(0,0,0)
    , _lastDotColor(.5, .5, .5)
    , _numDotSections(-1)
{

}


/**
 * Transforms the points of the polygon and draws it on the screen
 * 
 * @param    currentWorld    a reference to the world the object is in
 */

void Polygon::draw(World* currentWorld){

    // Some long and overly complicated method that should not apply here

}

/**
 * Adds a point to the polygon
 * 
 * @param    point        the point to be added
 * @param    color        the color to start at this point
 * @param    dotColor    the dotted line color to start at this point
 */
void Polygon::addPoint(Point point, RGB color, RGB dotColor){

    // Add the values to the lists
    _points.push_back(point);
    _colors.push_back(color);
    _dotColors.push_back(dotColor);

    // Update the last colors
    _lastColor = color;
    _lastDotColor = dotColor;
}

/**
 * Adds a point to the polygon
 * 
 * @param    point        the point to be added
 * @param    color        the color to start at this point
 */
void Polygon::addPoint(Point point, RGB color){
    // Add the point using the last dotted line color
    addPoint(point, color, _lastDotColor);
}

/**
 * Adds a point to the polygon
 * 
 * @param    point        the point to be added
 */
void Polygon::addPoint(Point point){
    // Add the point using the last line and dotted line color
    addPoint(point, _lastColor, _lastDotColor);
}

/**
 * Set the color of the current line
 * 
 * @param    color    the color of the line to be added
 */
void Polygon::setColor(RGB color){
    // Add the color to the list
    _colors.back() = color;

    // Update the last used color
    _lastColor = color;
}

/**
 * Set the dotted line color of the current line
 * 
 * @param    color    the dotted line color to be added
 */
void Polygon::setDotColor(RGB color){

    // Add the dotted line color to the list
    _dotColors.back() = color;

    // Update the last used dotted line color
    _lastDotColor = color;
}


/**
 * Sets the number of dotted sections for the current shape
 * 
 * @param    number    the number of dotted sections
 */
void Polygon::setDotSections(int number){
    _numDotSections = number;
}

// I just put this in to see if the issue was copying, but this text is never output
// so I don't believe it's being called 
Polygon::Polygon(const Polygon& rhs){

    std::cout << "Polygon copied" << std::endl;

    std::cout << "RHS: " << rhs._points.size() << " LHS: " << getPoints() << std::endl;
}

int Polygon::getPoints(){
    return _points.size();
}
compuguru
  • 1,035
  • 1
  • 10
  • 20
  • 1
    Names beginning with underscores are discouraged because several cases where one might use them cause undefined behavior. (Those identifiers are reserved for use in standard libraries). – Billy ONeal Feb 21 '11 at 03:33
  • 3
    The full definition of the `Polygon` class would be quite helpful to diagnosing the problem. As a rule, it's best to post a minimal example that demonstrates the problem, but is still sufficiently complete that it can be compiled as-is. Without that, it's hard to guess what important detail you may have omitted from your description. – Jerry Coffin Feb 21 '11 at 03:34
  • @Billy I thought I read somewhere that it's good to use underscores to denote class member variables, but based on the comments here I guess it's a bad idea. – compuguru Feb 21 '11 at 04:01
  • @compuguru: Some people discourage them, but leading underscore followed by lowercase letter is perfectly fine for data members. It's even preferred by other people. – Thomas Edleson Feb 21 '11 at 04:12
  • As long as you can make all your code in a consistent style, it works. Whatever is fine. :) – albert Feb 21 '11 at 11:43
  • @compuguru: Underscoes yes, but on the end of the member name, not the beginning. @Thomas: Technically it's not undefined here because it's not in the global namespace. But why force someone to have to pay attention to what exactly the rules are when it's easier just to not put underscores on the front in the first place? IF you want to denote using underscores that's fine, but you should put them on the end. – Billy ONeal Feb 21 '11 at 16:58
  • @albert: There are a few restrictions. For example, no name beginning with an underscore may be in the global namespace. No name may begin with an underscore followed by a capital letter (in any namespace). No name may begin with two underscores (in any namespace). If you're writing for a POSIX box, [POSIX defines a few more rules](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier/228797#228797). To break any of these rules results in implementation defined behavior. – Billy ONeal Feb 21 '11 at 17:04

5 Answers5

1
Rectangle playerPolygon = Rectangle(...);

You're invoking the copy constructor here. Did you write a copy constructor? Is it copying _points? If not, that's a bug; and there's really no need to force another copy constructor call in any case. Why not just

Rectangle playerPolygon(...);

? (But do watch out for the vexing parse...)

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
  • You have a good point there, but changing it to `Rectangle playerPolygon(...)` didn't resolve the issue. – compuguru Feb 21 '11 at 03:49
1

I don't see a reason. But I strongly suggest that you delete some lines in the constructors for copying objects. Also, the default constructor is not necessary to be in the constructor initialization list.

// This is just for reference, there are a few other classes involved
// but I don't think they should effect this.
Polygon::Polygon() 
    : _lastColor(0,0,0)
    , _lastDotColor(.5, .5, .5)
    , _numDotSections(-1)
{
}

How do you implement getPoints()?

albert
  • 364
  • 3
  • 8
  • Ah, ok, I've made those changes, looks a bit cleaner now. getPoints is pretty simple, all it does is `return _points.size()` (_points is private so if I use this to view the value in the main program) – compuguru Feb 21 '11 at 03:52
  • Do you define a copy constructor for Rectangle, such as Rectangle(Rectangle const &v)? Delete that copy constructor then. The default copy construct does a member wise initialization. Although there does not seem to be a copy would happen. But it might depend on compiler, for this code, _Rectangle playerPolygon = Rectangle(Point(1,1,-.75), Point(-1,-1,-.75), RGB(1,1,1));_, you'd write it something like, _Rectangle playerPolygon(Point(1,1,-.75), Point(-1,-1,-.75), RGB(1,1,1));_ – albert Feb 21 '11 at 04:02
  • I did have a copy constructor in there, but it was setup to print out text so I could see if it gets called (the text never gets printed though). I'm using the g++ compiler, but doing `Rectangle playerPolygon = Rectangle(...)` or `Rectangle playerPolygon(...)` doesn't make a difference. – compuguru Feb 21 '11 at 04:14
1

After sleeping on it, I figured it out! Being used to Java, I had made what I thought were overloaded constructors:

Rectangle::Rectangle(Point p1, Point p2, RGB lineColor, double dotSeg, RGB dotColor) : Polygon(){
    // Add some points here
}

Rectangle::Rectangle(Point p1, Point p2){
            // Supposed to call the first constructor
    Rectangle(p1, p2, RGB(), -1, RGB());
}


Rectangle::Rectangle(Point p1, Point p2, RGB lineColor){
            // Supposed to call the first constructor
    Rectangle(p1, p2, lineColor, -1, lineColor);
}

But then I tried using the main constructor in my program and it worked! After doing a bit of research, it turns out that constructors cannot be overloaded like this. When a constructor other than the original one is called, it creates a new Rectangle with the points, then destructs it right away because it's not set equal to anything and returns a new rectangle without the points.

I found the easiest way around this is to use default parameters, so I have the same functionality

Rectangle(Point p1, Point p2, RGB lineColor = RGB(), double dotSeg = -1, RGB dotColor = RGB());

I suppose this would have been caught if I put the entire code in there right away, but thanks for all the help and suggestions!

compuguru
  • 1,035
  • 1
  • 10
  • 20
0

A wild guess: Rectangle inherits from Polygon, and the Rectangle class declares a second member variable named _points that hides the base class version. Rectangle's constructor then adds some elements to Rectangle::_points, and some other function later checks the size of Polygon::_points and finds none.

If this is the case, just take away the declaration of _points in Rectangle. It needs to use the base class version, either directly if Polygon::_points is accessible (public or protected), or via accessor functions.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
0

A few hints that don't answer your question, but nonetheless:

  • I'm not sure why you derive Rectangle from Polygon. Unless you plan on taking advantage of polymorphism with the two types then it's not necessary and can even lead to incorrect code. The same holds for deriving Polygon from Object.
  • Don't use names that start with an underscore. The standard reserves this sort of names for implementors.
  • There's no need to explicitly initialize members with their default values. The compiler does that for you. For example, the statement _points = vector<Point>(); in the default Polygon constructor is redundant. Also, the call to the default constructor of Object() in that same Polygon constructor is redundant.
wilhelmtell
  • 57,473
  • 20
  • 96
  • 131
  • You've got some good points there. I am taking advantage of Polymorphism. Polygon contains methods used to draw a set of points. Rectangle is just a more specific set of points, so it uses the same draw method. I'll take care of the other 2 things you mentioned though. – compuguru Feb 21 '11 at 04:09