Qwt 6.0.0 and removal of copy constructors.

Hi Uwe,

I posted this on the mailing group, but thought I should have a go here too. Actually which method do you prefer? Mailing list or this forum?

Anyway...

I'm in the process of updating my code to Qwt 6.0.0. However, with most of the copy constructors gone it is making life very difficult. For example, previously I used to do this for a linear colour map:

Code:

---

SomeClass::editColourMap( QwtLinearColorMap* colourMap) {
  ... some code to edit the linear colour map by setting new colour stops. Then,
  scaleWidget_->setColorMap( QwtDoubleInterval( 0.0, 1.0), *colourMap->copy()); }

---

While I can do this is the new version:

Code:

---

scaleWidget_->setColorMap( QwtInterval( 0.0, 1.0), colourMap);

---

The original variable "colourMap" may not be persistant ... So I need to make a copy first. However, I can't do this:

Code:

---

scaleWidget_->setColorMap( QwtInterval( 0.0, 1.0), new QwtLinearColorMap( colourMap));

---

because there is private data within the class that is accessed via a pointer. The only thing I can think of to do is something like this:

Code:

---

QwtLinearColorMap* copyMap = new QwtLinearColorMap( origColourMap.color1(), origColourMap.color2());
  QVector<double> colourStops = origColourMap->colorStops();
  for ( int i = 1; i < colourStop.size() - 1; ++i) {
    copyMap->addColorStop( colourStop(i), blah_colour_from_rgb_lookup); // You get the idea.
  }
  scaleWidget_->setColorMap( QwtInterval( 0.0, 1.0), copyMap);

---

Any suggestions on this? How do I copy a colour map?

Have Fun !!

Shane

---

Added after 21 minutes:

Hi Uwe,

Just looking at the QwtLinearColorMap and QwtColorMap classes more closely and I'm note sure why you have gone for using a pointer to the class "PrivateData". All the members of PrivateData are well behaved in that none of the variables require the use of a user defined copy constructor to copy the data. The default C++ copy constructor will work fine. However, because QwtLinearColorMap uses a pointer to this data, you would need to have a user defined copy constructor. For eample is you have a class:

Code:

---

class A {
 public:
   A() {
     myString = new char[16];
     strcpy( myString, "Hello There");
   };
  ~A() {
    delete myString;
  }
 
 private:
   char *myString;
}

---

then to make a copy of class A you would need to create a user defined copy constructor to copy the contents of myString to the new copy. However, if you were to do this:

Code:

---

class B {
 public:
   B( const Qstring &aString) {
     myString = aString;
   };
  ~B() {
  }
 
 private:
   QString myString;
}

---

Then you don't need to user defined copy constructor or to clean up any data in a destructor. Then the following would then be valid:

Code:

---

B b1( "Hello There"), b2;
B b3( b1);
b2 = b1;
// b2 and b3 are now copies of b1.

---

The same is true of any complex classes like std::vector, std::valarray, QVector, QString, etc.

I'm not sure what your problem is - you have already posted the code how to create and initialize a copy of a linear color map. Put a function around it, or derive from it and implement the copy constructor + assignment and comparison operators and that's it.

Concerning the PrivateData: if I would like to have a copy constructor for a class I would implement one. This is no argument for or against hiding class members. In case of the color maps there is no reason for not having copy constructors - beside, that I didn't write them yet.

Uwe

Ahhh, so is your plan to return the copy constructors at a later date? That would be perfect.

My main point is that if rather than making your private data a pointer to another class that needs to be created/destroyed, just make the private data/class a direct member of the parent class and in about 95% of your code you would not need to create your own copy constructors or equals operator functions at all. It would just work as is using the the default C++ behaviour. Code is much easier to write and read if in client code if you can just write:

Code:

---

myLinearColourMap = someOtherLinearColourMap

---

rather than:

Code:

---

QwtLinearColorMap* copyMap = new QwtLinearColorMap( origColourMap.color1(), origColourMap.color2());
  QVector<double> colourStops = origColourMap->colorStops();
  for ( int i = 1; i < colourStop.size() - 1; ++i) {
    copyMap->addColorStop( colourStop(i), blah_colour_from_rgb_lookup); // You get the idea.
  }
  scaleWidget_->setColorMap( QwtInterval( 0.0, 1.0), copyMap);

---

Quote:

---

Ahhh, so is your plan to return the copy constructors at a later date?

---

To be honest I have no plans about this. When I don't forget about it it might happen, but we are talking about 4-5 lines of code, that could be written in the application code as well. So this is no strong reason for breaking binary compatibility.

And also I'm not sure about it at all - I don't like copy/assignment operations for classes with virtual interfaces.

Quote:

---

My main point is that if rather than making your private data a pointer to another class that needs to be created/destroyed, just make the private data/class a direct member of the parent class and in about 95% of your code you would not need to create your own copy constructors or equals operator functions at all.

---

The reason behind all this hiding of private members is binary compatibility. For a library with many users this is much much more important than to avoid writing some copy constructors and operators !

Think about the maintainers of a linux distribution and how many packages need to be rebuilt when a simple bug fix breaks binary compatibility of a low level library.

Uwe

Quote:

---

And also I'm not sure about it at all - I don't like copy/assignment operations for classes with virtual interfaces.

---

I've been working with this style of code a long while and have not had any serious issues yet.

Quote:

---

The reason behind all this hiding of private members is binary compatibility. For a library with many users this is much much more important than to avoid writing some copy constructors and operators !

---

Yep, good point. I can see the advantage of binary compatibility. So as long as the PrivateData is well behaved the parent class copy constructor hopefully could be a simple as *thisPrivateData = *otherPrivateData ... or even *thisPrivateData = new PrivateData( *otherPrivateData).

I'll have a play with a few of your classes that I specifically need copy constructors for and send you the code as an example. I'm also happy to spend some time and update all the classes if you're happy to go down that path and need the extra help. Otherwise, as you said earlier, I'll just create some wrapper classes or functions to handle the copying of classes outside of Qwt.

Shane

---

Added after 1 10 minutes:

Hi Uwe,

I moved the private definitions of the following to the public section and I tried this out in qwt_color_map.cpp:

Code:

---

QwtLinearColorMap::QwtLinearColorMap( const QwtLinearColorMap &colorMap)
{
    d_data = new PrivateData( *colorMap.d_data);
}
 
QwtLinearColorMap& QwtLinearColorMap::operator=( const QwtLinearColorMap &colorMap)
{
    if ( this != &colorMap) {
        QwtColorMap::operator=( colorMap);
        *d_data = *colorMap.d_data;
    }
    return *this;
}

---

So then to test, I modified the example spectrogram.cpp file to add in a copied class by setting:

Code:

---

d_spectrogram->setColorMap(new QwtLinearColorMap( ColorMap()));
...
QwtLinearColorMap *cmap = new QwtLinearColorMap();
*cmap = ColourMap();
rightAxis->setColorMap( zInterval, cmap);

---

so that I was taking a copy of ColourMap() and all worked correctly. If I remove ColorMap() out of these lines, then you get the default yellow to blue spectrogram.

Shane