Reasons for corrupt class member values?

[goatslayer]

I am passing a single integer to a constructor function call from another class, for some reason the value is corrupted when a method belonging to that class comes to use it.

Has anyone seen this before? - it is also happening when I use more arguments, but thought I'd test with just one.

On first look it seems the arg is corrupt, however when I assign the value to the class member and do a quick cout it seems that it has assigned it correctly.

I'm using the latest Mingw compiler hooked up to Dev-C++ 4.9.2.

[Paul McKenzie]

I am passing a single integer to a constructor function call from another class, for some reason the value is corrupted when a method belonging to that class comes to use it.

Please don't describe what you're doing, post the code.

Has anyone seen this before?

Yes. It's caused by bad programming, or misreading the results of whatever tool you're using to inspect variables.

On first look

What do you mean by "first look"? What are you using to look at these variables?

however when I assign the value to the class member and do a quick cout it seems that it has assigned it correctly.

So then there is no problem?

Regards,

Paul McKenzie

[goatslayer]

Thats just a bit rude paul, I felt I would save posting 130 lines of code in favour of a quick description, and why call it "bad programming", there might be a mistake on my part but I don't consider myself to be a bad programmer, I only posted because I don't see a mistake on my part.

I'll get the code ready to post, in the mean time there is a problem, the cout I do is immediately after assigning it in the constructor to see if the variable is holding the correct value, but a later cout, when calling any function from the class for the first time and the variable is holding some ridiculous value.

[goatslayer]

Class:

#include <iostream>
#include <cmath>
#include <vector>

#define PI 3.14159265

using namespace std;

class Polygon {

   public:

   struct coords {              //Structure for storing our coordinate values.
      double x;            
      double y;
   };

   Polygon(int sides) {    //poly constructor
      radius = 40;
      sides = sides;
      centerx = 320;
      centery = 240;
      orientation = 0;
      
      cout << "sides: " << sides << endl;
               
   }
   
   ~Polygon() { cout << "Polygon destructor called." << endl; }    //poly destructor

   int getSides() const {
       
       return sides;
   }

   double polarorient(double x) {    //rotate the polar coord grid 90 degrees to make
                                     //more transparent for every day usage, i.e.
      x = x+=90;                     //zero degrees matches zero on a protractor.
      if (x >= 360) {                //NOTE: Cyclical for multiples of 360.
         int test = (int)x/360;
         x-=(360*test);              //minus 360 times the no. 360's in that no.
      }
      return x;       
   }

   double cartx(double r, double theta) {    //return cartesian x from r, theta
    
       //polar coordinates taking 0,0 as center. (translate later to actual center)
       if (theta == 90) {    //cos 90 is 0 along the Y axis, but returns funny value 
          return 0;          //using calc, cmath and other methods so override.
       } else {
          double x;
          x = r * cos((theta) * PI/180);
          return x;
       }
   }

   double carty(double r, double theta) {    //return cartesian y from r, theta

      //polar coordinates taking 0,0 as center. (translate later to actual screen center)
      double y;
      y = r * sin((theta) * PI/180);
      return y;        
   }

   double removesign(double x) {    //change negative to non-negative number for
                                    //translation to screen coordinates.
      if (x < 0) {
         x=-x;
      } 
      return x;     
   }

   coords translatecoords(double angle, coords center, coords xy) {    //translate to center coords
                                                                       //based on splitting polar
      coords translatecoords;                                          //grid into 4 quadrants.

      if (angle >= 0 && angle <= 90) { 
         translatecoords.x = (int)((center.x + xy.x)+0.5);
         translatecoords.y = (int)((center.y - xy.y)+0.5);
      } else {
         if (angle >= 90 && angle <= 180) {
            translatecoords.x = (int)((center.x - xy.x)+0.5);
            translatecoords.y = (int)((center.y - xy.y)+0.5);
         } else {
            if (angle >= 180 && angle <= 270) {
               translatecoords.x = (int)((center.x - xy.x)+0.5);
               translatecoords.y = (int)((center.y + xy.y)+0.5);
            } else {
               if (angle >= 270 && angle <= 360) {
                  translatecoords.x = (int)((center.x + xy.x)+0.5);
                  translatecoords.y = (int)((center.y + xy.y)+0.5);
               }
            }
         }
      }       
      return translatecoords;       
   }

   vector <coords> vpoly() {

      cout << "into func testing var: " << sides << endl;

      centerpoint.x = 320;
      centerpoint.y = 240;   
      orientation = polarorient(orientation);
      centangle = 360/sides;

      for (count = 0; count < sides; count++) {
         polycoords.x = removesign(cartx(radius, orientation));    //gives raw points center 0,0 need translation
         polycoords.y = removesign(carty(radius, orientation));    //gives raw points center 0,0 need translation
         
         vpolycoords.push_back(translatecoords(orientation, centerpoint, polycoords));

         orientation+=centangle;      
         if (orientation > 360) { orientation-=360; }  
      }

      return vpolycoords;

   }

   private:
   
      int radius;
      int sides;
      int centerx;
      int centery; 
      int count;
      double orientation;
      double centangle;
      coords polycoords;  
      coords centerpoint;
      vector <coords> vpolycoords;

};

Application:

#include "pentclass.h"

   struct coords {
      double x;             
      double y;
   };

int main() {
   Polygon polly(5);
   std::cout << "testing sides: " << polly.getSides() << endl;         
   return 0;   
}

The class is still a work in progress, it is a class that returns the coords of the points of any regular polygon, I'm testing on a pentagon, I've modified the constructor so all that needs to be provided is the no. sides, and I've provided a getSides() method to return it's value. In the app all I call are the constructor and getSides(). Sides should come out from a cout in the constructor as 5, then in the cout in the app as 2089878893. - But this is incorrect and I can't see why.

[Philip Nicoletti]

In the constructor, either change the name of the argument or
change the assignment to:

this->sides = sides;

(I would choose changing the name of the argument).

[goatslayer]

Thanks Philip, worked a treat. I didn't think about changing the arg names.

I've found normally it is ok to have an argument the same name as a variable, in fact the practice is used in a couple of text books I've used, but I suppose it could be bad practice to do it this way.

Is there any reason the program would get confused in this case?

[Paul McKenzie]

Thats just a bit rude paul, I felt I would save posting 130 lines of code in favour of a quick description, and why call it "bad programming",

I wouldn't call it "good programming". There could be a myriad of reasons why code that doesn't work doesn't work, and 99% of those reasons point to the fault of the programmer. And "bad programming" doesn't mean that the person with the faulty code is a bad programmer. It means exactly what is stated -- something bad was done in the program.

As to being "rude" I guess these guys are rude to:

http://www.parashift.com/c++-faq-lit...t.html#faq-5.8

If you have a problem with code, no one can give an answer if no code is posted.

Describing in words what your code is doing seems, on the surface, to be helpful for someone not used to helping others figure out a problem, but in reality it isn't helpful, and many times winds up wasting time. I've seen too many times where a poster describes their code, and once their code is actually posted, they didn't do what they had described. Before posting the code, this led others giving wrong reasons for the code being wrong.

That's why it's always preferred to post the code that is giving the problem. You could have duplicated the problem with a much smaller example. The reason why reproducing the problem with a smaller example is beneficial is that maybe you will find the problem yourself when you attempt to reduce the program down before posting.

Regards,

Paul McKenzie

[Paul McKenzie]

Is there any reason the program would get confused in this case?

class foo
{
   public:
        void footest(int x) const { x = x;}
        int x;
};

int main()
{
   foo f;
   f.footest(1);
}

So which "x" is used? Is it the parameter value, or is it the member variable? The variable that is used is the parameter. Since the parameter is temporary, you just assigned a temporary to itself, not changing the member variable.

To prove this is the case, the footest() function is declared as const. This means that no member variable can be changed within the function. The code compiles with no errors at all, meaning that the member variable "x" was not changed.

This code does give an error:

class foo
{
   public:
        void footest(int x) const { this->x = x;}
        int x;
};

int main()
{
   foo f;
   f.footest(1);
}

Since you are now changing the member variable, this code no longer compiles.

What textbooks shows assigning an argument to itself, and claim that the member variable is being changed?

Regards,

Paul McKenzie

[goatslayer]

It looks like the text is wrong, don't laugh at the title but it is McGrath - C++ Programming in Easy steps, P104.

It declares a setter method using the arg (int age), and declares the variable int age as a class member.

When it defines the method it uses the arg (int yrs). Using this parameter it can assign the value to the variable age no problem.

I should have picked this up. Thanks.