[lmi] Persistent book-control page selection

[Greg Chicares]

I'd like to ask for code review of the 20080812T1150Z change,
which makes book-control page selection persist within a session.
Users open a book control to set parameters, then close it and
examine the outcome, often repeating that two-step process many
times in succession. Generally, they want to reopen the book
control to the last selected page; requiring them to select it
manually breaks their train of thought.

The wx calls seem subtle enough to merit review.

1. I represent the selection as std::size_t because that's what
ChangeSelection() takes as an argument, and assign it from the
result of GetSelection() only if that result is not -1. Would it
be more tasteful to compare to, e.g., wxNOT_FOUND instead of -1?
Is the comparison considered necessary at all, or should
  book_control.ChangeSelection(event.GetSelection());
always be valid?

2. In a wxEVT_COMMAND_BOOKCTRL_PAGE_CHANGED handler, I update the
global variable holding the selection, by calling GetSelection()
on the event rather than the control, as recommended in the wx
documentation.

3. I set the page in a wxEVT_INIT_DIALOG handler. Is that the
ideal place to do it? I do this by calling ChangeSelection();
because that function doesn't generate page-change events, should
I explicitly call the lmi code that such events would trigger?

4. BTW, the entry for ChangeSelection() probably ought to be
alphabetized in the documentation, instead of coming at the
bottom, as here:

  http://docs.wxwidgets.org/2.8/wx_wxnotebook.html
  http://docs.wxwidgets.org/2.8/wx_wxtreebook.html

In case it makes this easier to review, the 20080812T1150Z
change is given below in 'cvs diff -U3' format.

Index: mvc_controller.cpp
===================================================================
RCS file: /sources/lmi/lmi/mvc_controller.cpp,v
retrieving revision 1.21
diff -U 3 -r1.21 mvc_controller.cpp
--- mvc_controller.cpp  7 May 2008 13:01:48 -0000       1.21
+++ mvc_controller.cpp  12 Aug 2008 11:48:31 -0000
@@ -53,6 +53,17 @@
 #include <wx/xrc/xmlres.h>
 #include <wx/wupdlock.h>

+#include <cstddef>  // std::size_t
+#include <cstring>  // std::strlen()
+
+namespace
+{
+/// Open new bookcontrol to page selected in most recent instance.
+///
+/// A page selection is maintained for each bookcontrol resource.
+
+std::map<std::string,std::size_t> last_selected_page;
+
 /// Custom event to trigger a call to SetFocus(). This action requires
 /// a custom event because wxFocusEvent does not change focus--it only
 /// notifies the affected windows that focus changes have occurred.
@@ -68,8 +79,6 @@
 /// and add a handler in the event table that invokes a function that
 /// calls SetFocus().

-namespace
-{
 wxEventType const wxEVT_REFOCUS_INVALID_CONTROL = wxNewEventType();
 } // Unnamed namespace.

@@ -87,6 +96,9 @@
 {
     model_.TestInitialConsistency();

+    char const* resource_file_name = view_.ResourceFileName();
+    LMI_ASSERT(0 != resource_file_name && 0 != 
std::strlen(resource_file_name));
+
     if(!wxXmlResource::Get()->LoadDialog(this, parent, view_.MainDialogName()))
         {
         fatal_error() << "Unable to load dialog." << LMI_FLUSH;
@@ -693,12 +705,20 @@
         ,&MvcController::UponUpdateUI
         ,wxXmlResource::GetXRCID(view_.MainDialogName())
         );
+
+    
BookControl().ChangeSelection(last_selected_page[view_.ResourceFileName()]);
 }

 void MvcController::UponPageChanged(wxBookCtrlBaseEvent& event)
 {
     event.Skip();

+    int z = event.GetSelection();
+    if(-1 != z)
+        {
+        last_selected_page[view_.ResourceFileName()] = z;
+        }
+
     ConditionallyEnable();
 }