Opened 8 years ago

Last modified 7 years ago

#14145 confirmed optimization

wxAuiManager should take care about its own destruction

Reported by: Hanmac Owned by:
Priority: low Milestone:
Component: wxAui Version: stable-latest
Keywords: Cc: bwilliams
Blocked By: Blocking:
Patch: yes

Description

i have maked an patch for AuiManager so it can detect the destruction of the Window with the EvtDestroy.

Attachments (1)

auimanager.patch download (1.7 KB) - added by Hanmac 8 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 8 years ago by vadz

What happens to the existing code calling UnInit() from the frame dtor? Wouldn't it result in asserts from RemoveEventHandler() now?

Also, why compare the frame IDs instead of comparing event.GetEventObject() with m_frame, this seems to be less error-prone.

Changed 8 years ago by Hanmac

comment:2 Changed 8 years ago by Hanmac

i changed the patch file with your suggestions

comment:3 Changed 8 years ago by vadz

  • Cc bwilliams added
  • Status changed from new to confirmed
  • Summary changed from AUIManager should take care about its own destruction to wxAuiManager should take care about its own destruction

Thanks!

Does anybody have a problem with doing this? I don't have much experience with wxAUI but doing it like this looks logical to me, why should the frame need UnInit() manually when it can be done automatically (I'd remove the code doing this from the sample too BTW if this patch is applied)?

Any objections?

comment:4 Changed 8 years ago by biwillia76

Patch looks good.

comment:5 Changed 8 years ago by rk

No objections here. But we could remove the UnInit() method from the public api now or at least deprecate it and mention in the documentation that it is no longer necessary to call it.

comment:6 Changed 7 years ago by mmacleod

Can't see any reason not to apply this.

However I disagree with UnInit() being either deprecated or removed from the public API, other then the fact that this forces people to update code immediately (where they don't really need too)
I also don't think it is a 100% given that there is no use case still out there where someone might need/want to UnInit() manually (I can't think of one currently but that does not mean one does not exist), this function is hardly a maintenance burden or anything like that (as it is needed internally anyway) so I simple don't see what the positive of doing this would be.

Note: See TracTickets for help on using tickets.