Opened 12 years ago

Closed 6 years ago

#978 closed defect (fixed)

wxIconizeEvent::Iconized() should be GetIconized()

Reported by: hajokirchhoff Owned by:
Priority: low Milestone:
Component: GUI-all Version:
Keywords: wxShowEvent Cc: hajokirchhoff, Peter_Most@…
Blocked By: Blocking:
Patch: yes

Description

Maybe nitpicking, but I find consistent naming to be very
important the more class libraries I use. Thats one more
area where wxWindows is far superior to MFC.

wxShowEvent implements bool GetShow() but with
wxIconizeEvent the relevant function is Iconized().

I'd prefer GetIconized() or perhaps GetIconize()

Attachments (1)

ticket_978_patch download (3.5 KB) - added by Peter Most 6 years ago.
Patch for ticket 978

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by vadz

I'd rather prefer IsShown() and IsIconized() in both cases
so this is clearly subjective, but I agree that it would be
nice to be consistent... What do the others think about
this? Any opinions? We could easily add new names (keeping
the old ones for compatibility, of course) but we have to do
it before 2.6.0

comment:2 Changed 7 years ago by wojdyr

  • Component set to GUI-all
  • Keywords wxShowEvent added

and I can't find any documentation about wxShowEvent and wxEVT_SHOW

comment:3 Changed 6 years ago by vadz

  • Cc vadz removed
  • Status changed from new to confirmed

As I wrote before, I think GetShow() is a really poor name and IsShown() would have been better. If you do add the new names, please keep the old ones but mark them as deprecated, e.g.

#ifdef WXWIN_COMPATIBILITY_2_8
  wxDEPRECARED( bool GetShow() const { return IsShown(); } )
#endif

and add @deprecated to their docs in interface/event.h (the new names would need to be documented too, of course).

TIA!

Changed 6 years ago by Peter Most

Patch for ticket 978

comment:4 Changed 6 years ago by Peter Most

Made a patch with the following changes:

  • Marked wxShowEvent::GetShow() as deprecated (source and documentation)
  • Marked wxIconizeEvent::Iconized() as deprecated (source and documentation)
  • Added wxShowEvent documentation.
  • Added wxShowEvent::IsShown() implementation and documentation.
  • Added wxIconizeEvent::IsIconized() implementation and documentation.

HTH Peter

comment:5 Changed 6 years ago by Peter Most

  • Patch set

comment:6 Changed 6 years ago by Peter Most

  • Cc Peter_Most@… added

comment:7 Changed 6 years ago by vadz

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

Thanks, applied as r54320.

Just one small request: could you please submit patches with .patch or .diff extensions? This would allow Trac to highlight them.

Note: See TracTickets for help on using tickets.