Re: [lmi] Input-sequence editor testing

[Václav Slavík]

On 1 Sep 2010, at 20:09, Vaclav Slavik wrote:
> One thing to do is to change the duration_num_field() into wxSpinCtrl
> with limited range, and I will do that.

Here's updated version of the patch, with this and another minor fix (extra 
newline in error messages) implemented.

Regards,
Vaclav


diff --git a/input_sequence_entry.cpp b/input_sequence_entry.cpp
index 32dba3c..e89672a 100644
--- a/input_sequence_entry.cpp
+++ b/input_sequence_entry.cpp
@@ -43,6 +43,7 @@
 #include <wx/combobox.h>
 #include <wx/dialog.h>
 #include <wx/sizer.h>
+#include <wx/spinctrl.h>
 #include <wx/stattext.h>
 #include <wx/textctrl.h>
 #include <wx/valtext.h>
@@ -234,9 +235,9 @@ class InputSequenceEditor
         return get_field<DurationModeChoice>(Col_DurationMode, row);
     }
 
-    wxTextCtrl& duration_num_field(int row)
+    wxSpinCtrl& duration_num_field(int row)
     {
-        return get_field<wxTextCtrl>(Col_DurationNum, row);
+        return get_field<wxSpinCtrl>(Col_DurationNum, row);
     }
 
     wxStaticText& then_field(int row)
@@ -260,8 +261,11 @@ class InputSequenceEditor
 
     int compute_duration_scalar(int row);
     void adjust_duration_num(int row);
+    void adjust_duration_num_range(int row);
 
     void update_diagnostics();
+    bool is_valid_value(wxString const& s);
+    wxString get_diagnostics_message();
 
     void UponValueChange(wxCommandEvent& event);
     void UponDurationModeChange(wxCommandEvent& event);
@@ -355,7 +359,7 @@ void InputSequenceEditor::sequence(InputSequence const& s)
 
         duration_mode_field(i).value(data.end_mode);
 
-        int dur_num;
+        int dur_num = 0;
         switch(data.end_mode)
             {
             case e_number_of_years:
@@ -380,7 +384,7 @@ void InputSequenceEditor::sequence(InputSequence const& s)
                 }
             }
 
-        
duration_num_field(i).SetValue(value_cast<std::string>(dur_num).c_str());
+        duration_num_field(i).SetValue(dur_num);
 
         if(data.value_is_keyword)
             {
@@ -421,19 +425,19 @@ std::string InputSequenceEditor::sequence_string()
             case e_attained_age:
                 {
                 s.append(" @");
-                s.append(duration_num_field(i).GetValue().c_str());
+                
s.append(value_cast<std::string>(duration_num_field(i).GetValue()));
                 break;
                 }
             case e_duration:
                 {
                 s.append(" ");
-                s.append(duration_num_field(i).GetValue().c_str());
+                
s.append(value_cast<std::string>(duration_num_field(i).GetValue()));
                 break;
                 }
             case e_number_of_years:
                 {
                 s.append(" #");
-                s.append(duration_num_field(i).GetValue().c_str());
+                
s.append(value_cast<std::string>(duration_num_field(i).GetValue()));
                 break;
                 }
             case e_maturity:
@@ -528,10 +532,9 @@ void InputSequenceEditor::insert_row(int new_row)
     sizer_->wxSizer::Insert(insert_pos++, from_label, flags);
     wxChoice *duration_mode = new(wx) DurationModeChoice(this);
     sizer_->wxSizer::Insert(insert_pos++, duration_mode, flags);
-    wxTextCtrl* duration_num = new(wx) wxTextCtrl(this, wxID_ANY, "", 
wxDefaultPosition, wxDefaultSize, wxTE_RIGHT);
-    duration_num->SetValidator(wxTextValidator(wxFILTER_DIGITS));
+    wxSpinCtrl* duration_num = new(wx) wxSpinCtrl(this, wxID_ANY, "");
     sizer_->wxSizer::Insert(insert_pos++, duration_num, flags);
-    SizeWinForText(duration_num, "999", 20);
+    SizeWinForText(duration_num, "9999", 20);
     wxStaticText* then_label = new(wx) wxStaticText(this, wxID_ANY, 
LARGEST_THEN_TEXT);
     sizer_->wxSizer::Insert(insert_pos++, then_label, flags);
     SizeWinForText(then_label, LARGEST_THEN_TEXT);
@@ -613,19 +616,25 @@ void InputSequenceEditor::insert_row(int new_row)
     set_tab_order();
 
     // connect event handlers
-    value_ctrl->Bind
-        (wxEVT_COMMAND_TEXT_UPDATED
+    ::Connect
+        (value_ctrl
+        ,wxEVT_COMMAND_TEXT_UPDATED
         ,&InputSequenceEditor::UponValueChange
+        ,wxID_ANY
         ,this
         );
-    duration_mode->Bind
-        (wxEVT_COMMAND_CHOICE_SELECTED
+    ::Connect
+        (duration_mode
+        ,wxEVT_COMMAND_CHOICE_SELECTED
         ,&InputSequenceEditor::UponDurationModeChange
+        ,wxID_ANY
         ,this
         );
-    duration_num->Bind
-        (wxEVT_COMMAND_TEXT_UPDATED
+    ::Connect
+        (duration_num
+        ,wxEVT_COMMAND_SPINCTRL_UPDATED
         ,&InputSequenceEditor::UponDurationNumChange
+        ,wxID_ANY
         ,this
         );
 
@@ -737,8 +746,9 @@ void InputSequenceEditor::update_row(int row)
     // "maturity" should be an option only on the last row:
     duration_mode_field(row).allow_maturity(is_last_row);
 
-    // duration number visibility:
+    // duration number visibility and range:
     duration_num_field(row).Show(duration_mode_field(row).needs_number());
+    adjust_duration_num_range(row);
 
     if(duration_mode_field(row).value() == e_number_of_years)
         {
@@ -788,13 +798,10 @@ wxString InputSequenceEditor::format_from_text(int row)
         }
 
     duration_mode mode = duration_mode_field(row - 1).value();
-    long num = 0;
+    int num = 0;
     if(duration_mode_field(row - 1).needs_number())
         {
-        if(!duration_num_field(row - 1).GetValue().ToLong(&num))
-            {
-            return "";
-            }
+        num = duration_num_field(row - 1).GetValue();
         }
 
     switch(mode)
@@ -805,25 +812,24 @@ wxString InputSequenceEditor::format_from_text(int row)
             }
         case e_attained_age:
             {
-            return wxString::Format("from age %ld", num);
+            return wxString::Format("from age %d", num);
             }
         case e_duration:
             {
-            return wxString::Format("from duration %ld", num);
+            return wxString::Format("from duration %d", num);
             }
         case e_number_of_years:
             {
-            long yrs = 0;
+            int yrs = 0;
             int i = row - 1;
             while(0 <= i && duration_mode_field(i).value() == 
e_number_of_years)
                 {
-                long num_i = 0;
-                duration_num_field(i).GetValue().ToLong(&num_i);
+                int num_i = duration_num_field(i).GetValue();
                 yrs += num_i;
                 i--;
                 }
             return wxString::Format
-                ("%s + %ld years",
+                ("%s + %d years",
                 format_from_text(i + 1).c_str(),
                 yrs
                 );
@@ -862,17 +868,7 @@ T& InputSequenceEditor::get_field(int col, int row)
 
 int InputSequenceEditor::compute_duration_scalar(int row)
 {
-    long duration_num = -1;
-    wxString const duration_num_str = duration_num_field(row).GetValue();
-    if(duration_num_str.empty())
-        {
-        duration_num = 0;
-        }
-    else
-        {
-        duration_num_str.ToLong(&duration_num);
-        }
-    LMI_ASSERT(-1 != duration_num);
+    int duration_num = duration_num_field(row).GetValue();
 
     switch(duration_mode_field(row).value())
         {
@@ -915,6 +911,43 @@ int InputSequenceEditor::compute_duration_scalar(int row)
     throw "Unreachable--silences a compiler diagnostic.";
 }
 
+void InputSequenceEditor::adjust_duration_num_range(int row)
+{
+    if(!duration_mode_field(row).needs_number())
+        return;
+
+    int const prev_duration = (row > 0) ? duration_scalars_[row - 1] : 0;
+    wxSpinCtrl& duration = duration_num_field(row);
+
+    switch(duration_mode_field(row).value())
+        {
+        case e_attained_age:
+            {
+            duration.SetRange(input_.issue_age() + 1 + prev_duration, 
input_.maturity_age() - 1);
+            break;
+            }
+        case e_duration:
+            {
+            duration.SetRange(1 + prev_duration, input_.years_to_maturity() - 
1);
+            break;
+            }
+        case e_number_of_years:
+            {
+            duration.SetRange(1, input_.years_to_maturity() - prev_duration - 
1);
+            break;
+            }
+        case e_maturity:
+        case e_retirement:
+        case e_invalid_mode:
+        case e_inception:
+        case e_inforce:
+            {
+            fatal_error() << "unexpected duration_mode value" << LMI_FLUSH;
+            break;
+            }
+        }
+}
+
 void InputSequenceEditor::adjust_duration_num(int row)
 {
     int const scalar = duration_scalars_[row];
@@ -955,7 +988,9 @@ void InputSequenceEditor::adjust_duration_num(int row)
             }
         }
 
-    duration_num_field(row).SetValue(wxString::Format("%d", num));
+    adjust_duration_num_range(row);
+
+    duration_num_field(row).SetValue(num);
 }
 
 void InputSequenceEditor::update_diagnostics()
@@ -963,6 +998,48 @@ void InputSequenceEditor::update_diagnostics()
     // Validate the sequence and if it's not valid, show an error
     // and disable the OK button.
 
+    wxString msg = get_diagnostics_message();
+
+    if(diagnostics_->GetLabel() != msg)
+        {
+        diagnostics_->SetLabel(msg);
+        redo_layout();
+        }
+
+    ok_button_->Enable(msg.empty());
+}
+
+bool InputSequenceEditor::is_valid_value(wxString const& s)
+{
+    for(std::vector<std::string>::const_iterator k = keywords_.begin()
+       ;k != keywords_.end()
+       ;++k)
+        {
+        if(s == *k)
+            return true;
+        }
+
+    if(!keywords_only_)
+        return s.IsNumber();
+
+    return false;
+}
+
+wxString InputSequenceEditor::get_diagnostics_message()
+{
+    // Check some common problems and issue nice error messages for them:
+    for(int row = 0; row < rows_count_; ++row)
+        {
+        wxString const value = value_field(row).GetValue();
+        if(value.empty())
+            return wxString::Format("Missing value on row %d.", row);
+
+        if(!is_valid_value(value))
+            return wxString::Format("Invalid keyword \"%s\" on row %d.", 
value.c_str(), row);
+        }
+
+    // As fallback, parse the sequence and check the diagnostics. This may be
+    // less human-readable, but it's better than nothing at all:
     InputSequence const sequence
         (sequence_string()
         ,input_.years_to_maturity()
@@ -973,26 +1050,10 @@ void InputSequenceEditor::update_diagnostics()
         ,0
         ,keywords_
         );
-    std::string const diagnostics = sequence.formatted_diagnostics();
-
-    wxString new_diagnostics;
-
-    if(diagnostics.empty())
-        {
-        new_diagnostics = "";
-        ok_button_->Enable();
-        }
-    else
-        {
-        new_diagnostics = diagnostics.c_str();
-        ok_button_->Disable();
-        }
-
-    if(diagnostics_->GetLabel() != new_diagnostics)
-        {
-        diagnostics_->SetLabel(new_diagnostics);
-        redo_layout();
-        }
+    wxString msg = sequence.formatted_diagnostics().c_str();
+    // formatted_diagnostics() returns newline-terminated string, fix it:
+    msg.Trim();
+    return msg;
 }
 
 void InputSequenceEditor::UponValueChange(wxCommandEvent&)
diff --git a/wx_utility.hpp b/wx_utility.hpp
index 78fd6bc..6ed8b27 100644
--- a/wx_utility.hpp
+++ b/wx_utility.hpp
@@ -81,6 +81,7 @@ void Connect
     ,wxEventType     event
     ,Return (Class::*handler)(Argument)
     ,int             id = wxID_ANY
+    ,wxEvtHandler*   eventSink = NULL
     )
 {
     // Double parentheses: don't parse comma as a macro parameter separator.
@@ -100,7 +101,13 @@ void Connect
 
     typedef void (wxEvtHandler::*t0)(Argument);
     typedef wxObjectEventFunction t1;
-    object->Connect(id, event, c_cast<t1>(static_cast<t0>(handler)));
+    object->Connect
+        (id
+        ,event
+        ,c_cast<t1>(static_cast<t0>(handler))
+        ,NULL
+        ,eventSink
+        );
 }
 
 calendar_date ConvertDateFromWx(wxDateTime const&);