CODEWITHSUNDEEP
c++oopclass
Yingli Yan
Yingli Yan

Reputation: 69

Why didn't I initialize the private data member in class successfully?

Now, I faced with an exercise problem: Deal with some data in a class CRect. But my results are different from what I expected.

My code is as followed:

#include<iostream>
#include<cmath>
using namespace std;
class CRect
{
public:
    void Diagonal(CRect one)
    {
        float l;
        l=sqrt( (one.right-one.left)*(one.right-one.left)+(one.bottom-one.top)*(one.bottom-one.top) );
        cout<<"The length is "<<l<<endl;
    }
    void Show(CRect one)
    {
        cout<<"("<<one.left<<","<<one.top<<")"<<"   ";
        cout<<"("<<one.right<<","<<one.bottom<<")"<<endl;
    }
    CRect(float left1,float top1,float right1,float bottom1)
    {
        left=left1;
        top=top1;
        right=right1;
        bottom=bottom1;
    }
    CRect(float left1,float top1)
    {
        left=left1;
        top=top1;
    }
    CRect(CRect &c)           
    {
        right=c.right;
        bottom=c.bottom;
    }
private:
    float left,top,right,bottom;
};

int main()
{
    CRect r1(10,10,20,20);
    CRect r2(0,0);
    r2=CRect(r1);
    r1.Show(r1);
    r1.Diagonal(r1);
    r2.Show(r2);
    r2.Diagonal(r2);
    return 0;
}

Then，the result is as the following picture: the result picture

I think that I have not initialized the left and top . But, I don't know how to correct it. I can't find the mistake in my code.

Upvotes: 0

Views: 269

Answers (5)

DavidRR
DavidRR

Reputation: 19397

My answer doesn't directly address your question, but you'll want to get in the habit of using initialization lists with your constructors to initialize your class member member variables. (I say this recognizing that initialization lists may not yet have been introduced in your class.)

For example, instead of assigning values to your member variables from your constructor like this:

CRect(float left1, float top1)
{
    left=left1;
    top=top1;
}

...you should instead initialize your member variables like this:

CRect(float left1, float top1)
    : left(left1), top(top1)
{
}

For why, see the C++ FAQ entry [10.6] Should my constructors use "initialization lists" or "assignment"? as well as C++ initialization lists on SO.

In your own programming, you may also want to consider adopting a naming convention for your member variables to more easily distinguish them from other variable types. For example, instead of:

private:
    float left;       // no decoration

...consider using one of two common conventions:

private:
    float left_;      // trailing underscore
    float m_left;     // leading 'm_'

For more about this, see Trailing underscores for member variables in C++ on SO.

Upvotes: 0

juanchopanza
juanchopanza

Reputation: 227370

Your copy constructor only "initializes"1 right and bottom, top and left are left with garbage values:

CRect(CRect &c)           
{
    right=c.right;
    bottom=c.bottom;
}

Next, this two parameter constructor has similar flaws:

CRect(float left1,float top1)
{
    left=left1;
    top=top1;
}

You have to set all the data members to something. If you do so, you can completely remove the copy constructor and let the compiler synthesized one do its job.

1why the inverted commas? Because you are actually assigning values to data members that, at least semantically, have already been initialized. To initialize data members to a value, use the constructor initialization list:CRect(float left1,float top1) : left(left1), top(top1), right(), bottom() {}

Upvotes: 4

kassak
kassak

Reputation: 4184

Look at your copy constructor

CRect(CRect &c)           
{
    right=c.right;
    bottom=c.bottom;
}

should be

CRect(CRect &c)           
{
    right=c.right;
    bottom=c.bottom;
    top=c.top;
    left=c.left
}

Upvotes: 0

sbabbi
sbabbi

Reputation: 11181

Your copy constructor CRect(CRect &c) fails to initializer left and top. The copy constructor is called When you pass r1 and r2 to to the function Diagonal and Show.

Moreover:

  1. You don't need to define a copy-constructor at all. A default one is generated if you don't define your own.
  2. Show and Diagonal are member functions, they don't need to take a parameter.

Upvotes: 2

masoud
masoud

Reputation: 56479

You have not initialized all members in CRect(float left1,float top1) constructor and your copy-constructor.

CRect(const CRect &c) 
{
    right=c.right;
    bottom=c.bottom;
    top=c.top;
    left=c.left;
}

Or even omit your copy-constructor and allow default copy-constructor do everything.

More

CRect(float left1,float top1)
{
    left=left1;
    top=top1;
    // How about right and bottom ?!
}

Upvotes: 1

Related Questions