copy construct

HI!

I need to realize a didactic application, with a CFigure base class and 4 derivate classes: CLine, CTriangle, CCircle and CRectable.
The base clase (CFigure has 2 private special attributes:
CPen m_LineColor;
CBrush m_FillColor;) and the derivated classes acccess it throught public metods.
But, I have problems to the Set and Get methods of this classes.

CPen CFigure::GetLineColor()
{
EXTLOGPEN extlogpen;
m_LineColor.GetExtLogPen( &extlogpen );

return m_LineColor; //!!! Error HERE
}
Error: C2558: class Pen no copy constructor available

I build 2 constructors, a simple constructor and one with parameters.
CFigure::CFigure()
{
m_strFigName = _T("");
m_iX1 = 0;
m_iY1 = 0;
EXTLOGPEN extlogpen;
CPen line_col;
line_col.GetExtLogPen( &extlogpen );
LOGBRUSH LogBrush = { extlogpen.elpBrushStyle, extlogpen.elpColor, extlogpen.elpHatch };
m_LineColor.CreatePen( extlogpen.elpPenStyle, extlogpen.elpWidth, &LogBrush );

LOGBRUSH logbrush;
CBrush fill_col;
fill_col.GetLogBrush( &logbrush );
m_FillColor.CreateBrushIndirect(&logbrush);
}

CFigure::CFigure(int coordX, int coordY, CPen& line_col, CBrush& fill_col, CString figName)
{
m_strFigName = figName;
m_iX1 = coordX;
m_iY1 = coordY;

EXTLOGPEN extlogpen;
line_col.GetExtLogPen( &extlogpen );

LOGBRUSH LogBrush = { extlogpen.elpBrushStyle, extlogpen.elpColor, extlogpen.elpHatch };
m_LineColor.CreatePen( extlogpen.elpPenStyle, extlogpen.elpWidth, &LogBrush );

LOGBRUSH logbrush;
fill_col.GetLogBrush( &logbrush );
m_FillColor.CreateBrushIndirect(&logbrush);
}

CFigure::~CFigure()
{
m_LineColor.DeleteObject();
m_FillColor.DeleteObject();
}

What can I do to access this base class atribute (ex. m_LineColor)?

Your GetLineColor() routine wants to return a CPen object, but the compiler doesn't have the information it needs to copy a CPen object onto the stack, hence the error message. Copy constructors generally have the form, for example, CMyPen( const CMyPen &source_of_data ), and copy required data members from source_of_data to the invoking object.

Do a search for "copy constructor" in MSDN, and have a look at the articles. You can either write your own class based on CPen and include a copy constructor, or you can sidestep the whole problem and make m_LineColor public.

When returning from CFigure::GetLineColor, a copy of m_LineColor must be made. But, as compiler sais, class CPen has no copy constructor. You have a choice to derive from CPen and define a copy constructor, as TSYS already said.
Better is to return a reference from Get functions.
See next example:

Code:

---

class CFigure 
{
// ...
// Attributes
protected:
  CPen m_pen;
// Operations
public:
  // CPen GetPen1() {return m_pen;} // no copy constructor available
  CPen& GetPen2() {return m_pen;}
  const CPen& GetPen3() const {return m_pen;}
};

---

By the way... You really are Maximus Decimus Meridius, Commander of the Armies of the North, General of the Felix Legions, loyal servant to the true emperor, Marcus Aurelius. Father to a murdered son, husband to a murdered wife. ? :D

More to discuss here because your problem sounds like a quiz.

Declaring private members in base class and accessing them in derived classes throught public metods it's like
scratching yourself on the right side of head using the left hand. :)
Just declare them as protected, is there a problem for you to do this?

I have to disagree with ovidiucucu on this one. Returning a reference to an internal (aggregated) item is risky when there is no object managment involved.

If CFigure object goes out of scope, then the CPen object will be destroyed also. However there may be outstanding references (via ovidiucucu's suggestion). This will most likely lead to a crash or other undefined behaviour.

Scott Meyers has a full chapter (#23) devoted to this subject if you are looking for an indepth discussion.

Quote:

---

Originally Posted by TheCPUWizard

I have to disagree with ovidiucucu on this one. Returning a reference to an internal (aggregated) item is risky when there is no object managment involved.

If CFigure object goes out of scope, then the CPen object will be destroyed also. However there may be outstanding references (via ovidiucucu's suggestion). This will most likely lead to a crash or other undefined behaviour.

Scott Meyers has a full chapter (#23) devoted to this subject if you are looking for an indepth discussion.

---

At last, everithing is risky more or less. Because I'm not the first or the last suggesting (or implementing) that, you and Mr. Scott Meyers will have a lot to fight...
Do you think it's so difficult for a programmer to understand that keeping and using that reference to CPen after CFigure goes out of scope it's not good? Anyhow it's less probable to do that mistake.
Come on, let theoreticians write books and us continue programming.

Anyhow, you can read again my second post to see that the solution of OP is much more easier.

Thanks to everyone!

The problem was slove by me and I found a new other solution.

This subject is CLOSE.

As a professional developer I see the problem (using an obsolete/invalid reference) occur in the real world an average of 4-6 times a year. This does not could all of the times where the programmer having an "unexplained crash" or other strange behaviour finds the problemn himself!

Since a good number of hours (therefore $) are spend before I am called in, this indicates a fairly high real world cost related to returning references when you really need to return objects!

Of course if there is a method for tracking object usage (e.g. Reference Counting like COM), then you can pass references safely and freely, provided you always uuse the right type to hold the reference!

You are quote correct that the OP should make the PEN available to the derived classes using a much simpler technique. It is a tradeoff between making the CPEN itself a protected member or providing a protected accessor. I prefer the use of an accessor to remain consistant with the "data is private" paradigm and pure encapsulation.

Quote:

---

Originally Posted by TheCPUWizard

As a professional developer I see the problem (using an obsolete/invalid reference) occur in the real world an average of 4-6 times a year.

---

"There are three types of lies - lies, damned lies, and statistics"

Quote:

---

I prefer the use of an accessor to remain consistant with the "data is private" paradigm and pure encapsulation.

---

If you meet the perfect woman, I can tell, you'll don't like: she does not allow you all you wish. :)
This is one reason programmers love C++: it's not pure OO.
The pure OOP lovers can try Java, Smalltalk and so on...

Quote:

---

Originally Posted by ovidiucucu

Just declare them as protected, is there a problem for you to do this?

---

Protected is just as bad as public if you worry about breaking encapsulation.

Quote:

---

Originally Posted by ovidiucucu

This is one reason programmers love C++: it's not pure OO.
The pure OOP lovers can try Java, Smalltalk and so on...

---

And then again not everything in Java in an object, in pureOO spirit, for instance primitive types (int, float, double, etc.) ar not.

Quote:

---

Originally Posted by cilu

And then again not everything in Java in an object, in pureOO spirit, for instance primitive types (int, float, double, etc.) ar not.

---

Then enjoy with Smalltalk where True is a class having (among others) a wonderful method IsTrue returning self. (something similar with this).

Yes, I know. That was my point. :thumb: ;)

Quote:

---

Originally Posted by cilu

Protected is just as bad as public if you worry about breaking encapsulation.

---

Well, not quite as bad ;) . If a derived class gains a reference to an aggregated entity (either by an accessor or directly to the data), there is no issue of the actual object going out of scope before the reference as the derived class will always be destroyed before the base class. This is not true if other objects perform the same type of operation where order of destruction can not be known (by the designer of the base class in question).

Quote:

---

Originally Posted by TheCPUWizard

I have to disagree with ovidiucucu on this one. Returning a reference to an internal (aggregated) item is risky when there is no object managment involved.

If CFigure object goes out of scope, then the CPen object will be destroyed also. However there may be outstanding references (via ovidiucucu's suggestion). This will most likely lead to a crash or other undefined behaviour.

Scott Meyers has a full chapter (#23) devoted to this subject if you are looking for an indepth discussion.

---

Code:

---

class CFufu 
{
public:
  CFufu() : m_strFufu(_T("FUFU")) {}
private:
  CString m_strFufu;
};

class CFu
{
private:
  CFufu m_fufu;
public:
  CFufu& GetFufu() {return m_fufu;}
};

---

Please, give us one simple example how to make a reference to a CFufu object initializated via CFu::GetFufu(), that can be used after CFu object goes out of scope (refer to an invalid CFufu object, so generates a dangerous situation).

Code:

---

CFu *p = new CFu();
CFuFu &r = p->GetFuFu();
delete p;
// r is now invalid

---

Messing up with references is pretty simple. If we are going back to Scott Meyers, Chapter 1 from More Effective C++ should be clear enought:

Quote:

---

First, recognize that there is no such thing as a null reference. A reference must always refer to some object. As a result, if you have a variable whose purpose is to refer to another object, but it is possible that there might not be an object to refer to, you should make the variable a pointer, because then you can set it to null. On the other hand, if the variable must always refer to an object, i.e., if your design does not allow for the possibility that the variable is null, you should probably make the variable a reference.

References, then, are the feature of choice when you know you have something to refer to, when you'll never want to refer to anything else, and when implementing operators whose syntactic requirements make the use of pointers undesirable. In all other cases, stick with pointers.

---

Quote:

---

Originally Posted by AnotherAlias

Code:

---

CFu *p = new CFu();
CFuFu &r = p->GetFuFu();
delete p;
// r is now invalid

---

---

Well, nomore contradicting Scott Meyers, but just a note:
The above code demonstrates that if somebody really wants to do something bad, then for sure can do it.
Even much more natural is just to write...

Code:

---

  CFu fu;
  CFufu& rFufu = fu.GetFufu();
  // use rFufu without headaches!

---

...why make it easier and safer since it can be made complicated and dangerous? :)

Yes, but there are so many cases (pointless to give one), when

Code:

---

CFu fu;
CFufu& rFufu = fu.GetFufu();
// use rFufu without headaches!

---

just can't be used, thus

Code:

---

CFu *p = new CFu();
CFuFu &r = p->GetFuFu();
delete p;
// r is now invalid

---

is the only solution. And then you have to be very careful with those references...

Quote:

---

Originally Posted by cilu

Yes, but there are so many cases (pointless to give one), when

Code:

---

CFu fu;
CFufu& rFufu = fu.GetFufu();
// use rFufu without headaches!

---

just can't be used, thus

Code:

---

CFu *p = new CFu();
CFuFu &r = p->GetFuFu();
delete p;
// r is now invalid

---

is the only solution. And then you have to be very careful with those references...

---

IMHO: nothing is pointless. Please, just give one.

EDIT: Usually, I am very careful with those references. ;)
But I think, instead of learn by heart some rules, it's better to discuss them.
This one may be an interesting subject.

Of course any one can do bad things, but you can make it more difficult for them to do so.


Just a note: this whole disucssion is outside the original posters issue, since his can be solved by only exposing the CPen as a protected object, therefore it will only be available to derived classes which can not exist pas the point of the base class.


note: Cilu's post on when to use pointers, I completely agree with, but in this case changing the reference to a pointer does nothing to help the problem wince the desctruction of the owning object has no was to set any existing pointers which refer to its members to null

The simplest way to handle this is by using RCO's [reference counded objects]. There are two common patterns.

1) Have the containing class dynamically allocate the contained class so that it can exist longer than the container. [It would self destruct when its reference count goes to 0.

2) Have the container class be "bound into existance" by the contained class. This would have the effect of the contained object keeping an outstanding reference to the outter object. This is a more difficult pattern to implement, but is necessary in some cases.

If you ar going to use reference counting (or another object management paradigm) it is a good idea to be consistant and dynamically allocate all of your objects. While this takes a few more keystrokes, it gives provides a number of benefits.

Readers should recognize the information in this post as being exteremely similar to the techniques used by COM.

Quote:

---

Originally Posted by TheCPUWizard

Just a note: this whole disucssion is outside the original posters issue, since his can be solved by only exposing the CPen as a protected object, therefore it will only be available to derived classes which can not exist pas the point of the base class.

---

... as somebody already said.

Quote:

---

Originally Posted by ovidiucucu

Declaring private members in base class and accessing them in derived classes throught public metods it's like
scratching yourself on the right side of head using the left hand. :)
Just declare them as protected, is there a problem for you to do this?

---

However, you cannot blame us, as long as this new direction is opened by yourself:

Quote:

---

Originally Posted by TheCPUWizard

I have to disagree with ovidiucucu on this one. Returning a reference to an internal (aggregated) item is risky when there is no object managment involved.

If CFigure object goes out of scope, then the CPen object will be destroyed also. However there may be outstanding references (via ovidiucucu's suggestion). This will most likely lead to a crash or other undefined behaviour.

Scott Meyers has a full chapter (#23) devoted to this subject if you are looking for an indepth discussion.

---

Anyhow, I agree with this point of view:

Quote:

---

Originally Posted by ovidiucucu

IMHO: nothing is pointless.
...................................................................
I think, instead of learn by heart some rules, it's better to discuss them.
This one may be an interesting subject.

---

Thanks for the comments. I personally believe that it is a good thing when threads discuss why various solutiions are not ideal. This is good even if a better solution has already been found. All too often people see one method of doing something and then ask "why didn'y they......". By covering alternatives this can be addrssed up front.

Also the various FAQS and many articles are distilled from topics that involve alot of discussion. This is one of the biggest [IMHO] benefits of forums such as this.

Quote:

---

Originally Posted by ovidiucucu

IMHO: nothing is pointless. Please, just give one.

---

OK: when it comes to class hierarchies pointers are the beter choise.

I don't see how you can use

Code:

---

CFu fu;
// do something with fu

---

when fu is supposed to be a Decorator or part of a Strategy, etc.

As for going out of the topic, sometimes that happens and discussions like this one are very wellcomed IMHO.