Re: [lmi] Event handling modernization

[Greg Chicares]

On 2014-11-30 01:04, Vadim Zeitlin wrote:
[...]
>  The 0002-Don-t-count-all-MDI-children-just-check-if-there-is-.patch one
> addresses my urge to avoid performing unnecessary actions: we don't really
> need the count of MDI children frame here at all, so why bother counting
> all of them (even if "all" is probably virtually always just a few) when we
> can stop after just the second one? Subsidiarily, why count them at all if
> we don't have any active child anyhow.

This gives me pause:

http://docs.wxwidgets.org/trunk/classwx_m_d_i_parent_frame.html#a981df3eae1daa82772fe10e3bcba7215
| virtual wxMDIChildFrame* wxMDIParentFrame::GetActiveChild() const
| Returns a pointer to the active MDI child, if there is one.
| If there are any children at all this function returns a non-NULL pointer.

I take "any...at all" to mean that if the parent frame has no *MDI* children,
but does have a child window of some other type, then this function returns
a non-NULL wxMDIChildFrame* that must point to an object that is not actually
a wxMDIChildFrame. Maybe I'm reading too much into this, but it worries me:
wouldn't such a pointer be likely to be misused?

Suppose GetActiveChild() returns a pointer to a non-MDI child, and there is
at least one other child window that is an MDI child. (The "if there is one"
language seems to suggest that this is impossible, but that second sentence
makes me wonder if that's true.) In that case, we enable "Window | Next"
when we shouldn't. Well, at least we don't attempt to call any derived-class
function through this base-class pointer.

I'll just rule that out, at no real cost, thus:
     wxMDIChildFrame* child_frame = frame_->GetActiveChild();
-    if(child_frame)
+    if(dynamic_cast<wxMDIChildFrame*>(child_frame))

Since I rewrote this in order to understand it, let me also ask...here:

https://github.com/vadz/lmi/commit/461ba86312ed5409237652d19309a3b8f28ddeaa.patch
+        for(wxWindowList::const_iterator i = wl.begin(); i != wl.end(); ++i)
+            {
+            wxMDIChildFrame* const child = dynamic_cast<wxMDIChildFrame*>(*i);
+            if(child && child != child_frame)

Why
  wxMDIChildFrame* const child // the thing-pointed-to cannot be changed
instead of
  wxMDIChildFrame const* child // the pointer cannot be changed
? Was that a conscious, deliberate choice? I wrote the latter out of habit,
but is that a careless habit? Anyway, I'll commit
  wxMDIChildFrame const*const child // nothing can be changed
because I don't see any argument against it.