Re: [lmi] Not urgent: a plausible micro-optimization

[Greg Chicares]

On 2018-02-22 13:31, Vadim Zeitlin wrote:
[...]
> assignment of wxFrame::GetChildren() to std::list<> does bother me because
> this only compiles in STL build of wxWidgets and I'd like to be able to
> build lmi with the default build not using STL.

Oh. I had assumed that by now a default wx build would use STL, and
that the old containers were just kept for backward compatibility.

> 2. Replace the existing assignment with a copy. This is what I did in a
>    new https://github.com/vadz/lmi/pull/69

I'm not opposed to accommodating the wx container classes. But I'd
rather not use a for-loop to bring one element to the front [and I
don't think that's exactly the loop you intended to write anyway].
How about the following alternative?

---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
diff --git a/skeleton.cpp b/skeleton.cpp
index dc0d61015..11d3768cc 100644
--- a/skeleton.cpp
+++ b/skeleton.cpp
@@ -1369,9 +1369,9 @@ void Skeleton::UpdateViews()
 {
     wxBusyCursor wait;
 
-    // Assignment implicitly ensures that this is not a wx legacy
-    // container, and thus that std::list operations are valid.
-    std::list<wxWindow*> z = frame_->GetChildren();
+    // Make a local copy of the list for modification.
+    wxWindowList const&  y = frame_->GetChildren();
+    std::list<wxWindow*> z(y.begin(), y.end());
     wxMDIChildFrame*     a = frame_->GetActiveChild();
     // Bring any active child to front so it's updated first.
     // It doesn't matter here if it's null: that's filtered below.
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------

AIUI, all std::list operations are also defined for wxList, so
that we might use wxList above instead of the standard class,
but I hesitate to do that because I'm not really sure how
closely compatible the classes are.

BTW, this:
    z.remove(a);
    z.push_front(a);
seemed slightly ugly to me (remove() processes the whole list,
and we only need the first occurrence, which should be unique;
and some sort of swap function seems more fitting anyway), so
I considered doing this instead:
    std::iter_swap(z.begin(), std::find(z.begin(), z.end(), a));
However, in the "impossible" case that active child 'a' is not
an element of child-list 'z', that would do the wrong thing;
and if we assert against that "impossibility", then the code
becomes verbose and less clear.