Opened 10 years ago

Closed 2 years ago

#2146 closed enhancement (fixed)

Copy constructor for wxValidator

Reported by: hwiesmann Owned by:
Priority: low Milestone:
Component: GUI-generic Version:
Keywords: simple Cc: hwiesmann, abxabx
Blocked By: Blocking:
Patch: yes

Description

When implementing a new validator class the a new class has to
be derived from wxValidator. wxValidator requires to overload the
Clone function. A clone function is often implemented like:

wxObject* Clone(void) const {return new(*this);}

Therefore, the derived class has to implement a copy constructor.
Unfortunately, wxValidator does not have a copy constructor
implemented. Therefore, in the copy constructor of the derived
function, first, the default wxValidator constructor has to be called
and afterwards the copy function of wxValidator.
This looks very ugly. A copy constructor of wxValidator should
solve this issue.

Attachments (2)

2146.patch download (743 bytes) - added by oneeyeman 2 years ago.
Attempt to fix
2146.diff download (763 bytes) - added by oneeyeman 2 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 10 years ago by abxabx

To which wxWidgets version this report refers? I see in CVS
Head there is Clone in wxValidator so can I close this bug or do
you have any further notes?

ABX

comment:2 Changed 10 years ago by abxabx

Hartwig,

I repost my question: To which wxWidgets version this report
refers? I see in CVS Head there is Clone in wxValidator so
can I close this bug or do you have any further notes?

ABX

comment:3 Changed 9 years ago by hwiesmann

Hi ABX,

indeed there is a clone function in wxValidator but what is needed is a
COPY constructor.
As I mentioned before there are some ways to work around this
shortcoming but they are neither efficient nor nice.

Hartwig

comment:4 Changed 3 years ago by oneeyeman

It looks like this bug still stands.
There is no copy constructor in wxValidator. But I guess it can be done differently.

Can this be closed?

comment:5 follow-up: Changed 3 years ago by vadz

  • Keywords simple added
  • Priority changed from normal to low
  • Status changed from new to confirmed
  • Type changed from defect to enhancement

It would indeed make sense to add a copy ctor to wxValidator, no idea why this was never done but it should be.

comment:6 in reply to: ↑ 5 Changed 3 years ago by oneeyeman

Vadim,
Replying to vadz:

It would indeed make sense to add a copy ctor to wxValidator, no idea why this was never done but it should be.

Maybe because it uses wxDECLARE_NO_COPY_CLASS() macro?

Anyway do we want to remove the macro and implement copy constructor?

comment:7 Changed 3 years ago by vadz

Yes, this is exactly what I wrote in comment:5, didn't I? As an aside, I do know that it uses wxDECLARE_NO_COPY_CLASS() but its presence doesn't explain why is it there.

Changed 2 years ago by oneeyeman

Attempt to fix

comment:8 Changed 2 years ago by oneeyeman

  • Patch set

Attached please find the implementation of this request.
Please review and apply.

comment:9 Changed 2 years ago by oneeyeman

Sorry forgot to mention: patch is generated based on rev. 73355

comment:10 Changed 2 years ago by vadz

2 problems:

  1. Copy ctor shouldn't copy static fields, this doesn't make any sense.
  2. Before the class didn't have neither copy ctor nor assignment operator, now it has both while it probably still shouldn't have the latter. I.e. wxDECLARE_NO_COPY_CLASS() should be replaced with wxDECLARE_NO_ASSIGN_CLASS() instead of being removed.

Changed 2 years ago by oneeyeman

comment:11 Changed 2 years ago by oneeyeman

Attached please find version 2 of this patch.

comment:12 Changed 2 years ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [73403]) Add copy constructor to wxValidator.

It can be useful for implementing Clone() in the derived classes.

Closes #2146.

Note: See TracTickets for help on using tickets.