views:

176

answers:

4

hi im trying to pass some values to a class but it wont let me it says invalid use of class 'Figure' im trying to send 3 values x,y,z and thats all but it wont let me heres what im trying to do...

here is the main.cpp and the function that calls the class Figure

 for (j = 0; j < num_elems; j++) {
    /* grab and element from the file */
    vlist[j] = (Vertex *) malloc (sizeof (Vertex));
    ply_get_element (ply, (void *) vlist[j]);


    int vert=sprintf(szFile,"vertex: %g %g %g", vlist[j]->x, vlist[j]->y, vlist[j]->z);
    /* print out vertex x,y,z for debugging */
    TextOut(hDC,600,j*20,szFile,vert);
   DrawFig->Figure(vlist[j]->x, vlist[j]->y, vlist[j]->z);
  }

The error is here

   DrawFig->Figure(vlist[j]->x, vlist[j]->y, vlist[j]->z);
  }

Here is the WM_CREATE: where i initialize everything

case WM_CREATE:
        hDC = GetDC(hWnd);
     //ShowWindow(g_hwndDlg,SW_SHOW);
    hRC=wglCreateContext(hDC);
    wglMakeCurrent(hDC,hRC);
     g_hwndDlg = CreateDialog(hInst,MAKEINTRESOURCE(IDD_DIALOG1),hWnd,DialogProc);

    DrawFig= new Figure(1.0,1.0,1.0);
    initGL();
     break;

here is the Figure.h

class Figure
{
    public:
        Figure(float x,float y,float z);
        void Draw();
        float paramx(){
        return x1;
        }
        float paramy(){
        return y1;
        }
        float paramz(){
        return z1;
        }
    protected:
    private:
    float x1,y1,z1;
    list <Figure> m_vertices;
};

and here is the Figure.cpp

Figure::Figure(float x,float y,float z){
    this->x1=x;
this->y1=y;
this->z1=z;
m_vertices.push_back(Figure(x1, y1, z1));
}

void Figure::Draw()
{
    list<Figure>::iterator p = m_vertices.begin();
 glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

    glLoadIdentity();
    gluLookAt(0.0,0.0,4.0,0.0,0.0,0.0,0.0,1.0,0.0);

    glColor3f(0.7f,1.0f,0.3f);
    glBegin(GL_LINE_LOOP);
    while(p != m_vertices.end()){
        glNormal3f(p->paramx(),p->paramy(),p->paramz());
        glVertex3f(p->paramx(),p->paramy(),p->paramz());
        p++;
    }
    glEnd();
}

any ideas? this is opengl,c++ and im using codeblocks 10.05 just in case oh yeah im initializing it at the main.h like this DrawFig* Figure;

+2  A: 

You cannot call a constructor directly in the way you attempt to. Create a set() function that will do the same work and use it instead of the constructor:

class Figure  {
  // ...
public:
  void set(float x, float y, float z);
  // ...
};

void Figure::set(float x, float y, float z)
{
  // Your original code from the constructor
  this->x1 = x;
  this->y1 = y;
  this->z1 = z;
  // m_vertices.push_back(Figure(x1, y1, z1));
}

Figure::Figure(float x, float y, float z)
{
  // In the constructor call the newly created set function
  set(x, y, z);
}


// Replace the faulty line with this:
DrawFig->set(vlist[j]->x, vlist[j]->y, vlist[j]->z);

EDIT:

As noted in the comments, the code has yet another flaw - you have a list of figures that is contained within the Figure itself. I think you meant to declare m_vertices as follows:

list <Vertex> m_vertices;

Then, however, if you want a Figure to be a triangle (or any other higher-order polygon), you will need to pass coordinates of all three vertices instead of the three coordinates of one vertex:

void Figure::set(const Vertex& v1, const Vertex& v2, const Vertex& v3)
{
  m_vertices.push_back(v1);
  m_vertices.push_back(v2);
  m_vertices.push_back(v3);

  // The position of the figure will be its centroid
  this->x1 = (v1.x + v2.x + v3.x) / 3;
  this->y1 = (v1.y + v2.y + v3.y) / 3;
  this->z1 = (v1.z + v2.z + v3.z) / 3;
}

Figure::Figure(const Vertex& v1, const Vertex& v2, const Vertex& v3)
{
  set(v1, v2, v3);
}

You will also need to adjust the loop to read 3 vertices at once instead of only one but I'll let that up to you :)

dark_charlie
You can. :) See my answer.
Albert
This results in infinite recursion, and so isn't the correct answer. Your `set` method ends up calling the constructor to construct the new list element which calls `set` which ends up calling the constructor... etc... It's not only infinitely recursive, but you'll use up the whole heap too if you can with new list nodes.
Omnifarious
@Omnifarious Thanks for noticing this, I've completely overlooked that. Edited the post to cover this as well.
dark_charlie
out of 4 answers, 2 of them are referring to your answer. +1 for that :)
Default
+1  A: 

A few things:

  • Did you instantiate the Figure class?
  • Is the list <Figure> m_vertices; instantiated?

The usage of using C's malloc function with the C++ runtime code is messy, best to stick with new instead to keep the C++ runtime consistent.

tommieb75
hmm how do you instantiate the list ?
Makenshi
m_vertices if I have read this correctly is not initialized no? I thought there would be a line like 'm_vertices = new list<Figure>();' no?
tommieb75
+1 for mentioning malloc use in C++ code
dark_charlie
+2  A: 

Just about the direct constructor call:

Use this instead:

// destruct and reconstruct
DrawFig -> ~Figure();
new (DrawFig) Figure(vlist[j]->x, vlist[j]->y, vlist[j]->z);

What it does:

  1. It calls the destructor.

    The destructor itself will call the destructor of all member variables. floats don't need/have a destructor but std::list has. std::lists destructor will free all containing objects.

  2. It calls the constructor.

    The constructor itself will call the constructor of all member variables. Again, floats don't have that and they are not initialized in a specific way, i.e. they are ignored again. Then the constructor of std::list is called which will initialize the list.

However, using dark_charlie's solution might be more clean.


Not only is DCs solution more clean, it also does something different. By calling the constructor again, you would also reset Figure::m_vertices and I think this is probably not what you want here.

However, maybe instead of set (like in DCs solution) you should name it add or so instead.

Also I am not sure if you really want to have Figure or Figure::m_vertices that way (each Figure containing a list to other Figures).

Albert
I wouldn't teach a beginner such an approach. Yes, it's valid but I wouldn't want to read the code a few months or years later :)
dark_charlie
I hesitate to give you the +1 because this is distressing and ugly and will greatly confuse a new programmer. OTOH, learning exactly what's happening here will be very instructive for said programmer, hence the +1.
Omnifarious
Both of the answers resolved the problem... but now its not drawing anything XD
Makenshi
wait what do you mean by Also I am not sure if you really want to have Figure or Figure::m_vertices that way (each Figure containing a list to other Figures).
Makenshi
+2  A: 

@dark_charlie's answer is almost correct. Here is a better version that will actually work, but still probably isn't what you want:

class Figure  {
  // ...
public:
  void set(float x, float y, float z);
  // ...
};

void Figure::set(float x, float y, float z)
{
  // Your original code from the constructor
  this->x1 = x;
  this->y1 = y;
  this->z1 = z;
}

Figure::Figure(float x, float y, float z)
{
  // In the constructor call the newly created set function
  set(x, y, z);
  m_vertices.push_back(Figure(x1, y1, z1));
}


// Replace the faulty line with this:
DrawFig->set(vlist[j]->x, vlist[j]->y, vlist[j]->z);

Now, this is almost certainly not what you want. But it's also really hard to figure out what you do want. You have a design problem. The design problem is that Figure has two responsibilities. It is both a point in space, and a set of points describing a figure. This confusion of responsibilities is leading your class to not actually be able to fill either of them particularly well.

You need two classes. You need a Point class and a Figure class. The Figure class should allow you to set the location of the figure as well as letting you add points to the figure's outline.

The huge clue that something is wrong is this list<Figure> m_vertices;. It's very rare that a class conceptually contains instances of itself. And usually when you do it you're building your own data structure like a tree or a list and then the class contains pointers to instances of itself.

Also, the fact that @dark_charlie's simple fix resulted in infinite recursion is another huge clue that something is wrong.

I'm guessing this is a homework assignment, so this is all the help I will give you aside from telling you that I think you already have a Point class that you call Vertex.

Omnifarious
haha yeah its a homework and thank you im on to trying it now
Makenshi
+1 for narrowing this down :)
dark_charlie
ok thank you very much i think this is the best one lol
Makenshi