[lmi-commits] [lmi] odd/dtor-verifies-postcondition 03e8ed4: Demonstrate safe use of dtor to verify postconditions

[Greg Chicares]

branch: odd/dtor-verifies-postcondition
commit 03e8ed426e32c38a05777d14e5a8e5841d26b5c1
Author: Gregory W. Chicares <address@hidden>
Commit: Gregory W. Chicares <address@hidden>

    Demonstrate safe use of dtor to verify postconditions
    
    See contemporary mailing-list discussion.
---
 progress_meter.cpp      | 18 ++++++++++++++++--
 progress_meter.hpp      | 40 ++++++++++++++++++++++------------------
 progress_meter_test.cpp | 39 +++++++++++++++++++++++++++++----------
 3 files changed, 67 insertions(+), 30 deletions(-)

diff --git a/progress_meter.cpp b/progress_meter.cpp
index eb1e155..40dcfb4 100644
--- a/progress_meter.cpp
+++ b/progress_meter.cpp
@@ -26,6 +26,7 @@
 #include "alert.hpp"
 #include "timer.hpp"                    // lmi_sleep()
 
+#include <exception>                    // uncaught_exceptions()
 #include <sstream>
 
 std::ostringstream& progress_meter_unit_test_stream()
@@ -77,6 +78,14 @@ progress_meter::progress_meter
 {
 }
 
+progress_meter::~progress_meter()
+{
+    if(!std::uncaught_exceptions() && !are_postconditions_met())
+        {
+        safely_show_message("Please report this: culminate() not called.");
+        }
+}
+
 void progress_meter::dawdle(int seconds)
 {
     do_dawdle(seconds);
@@ -100,9 +109,9 @@ bool progress_meter::reflect_progress()
 void progress_meter::culminate()
 {
     culminate_ui();
-    if(!was_cancelled_ && max_count_ != count_)
+    if(!are_postconditions_met())
         {
-        warning()
+        alarum()
             << max_count_
             << " iterations expected, but only "
             << count_
@@ -133,3 +142,8 @@ void progress_meter::do_dawdle(int seconds)
     lmi_sleep(seconds);
 }
 
+bool progress_meter::are_postconditions_met() const
+{
+    return was_cancelled_ || max_count_ == count_;
+}
+
diff --git a/progress_meter.hpp b/progress_meter.hpp
index 634f00c..99b5e31 100644
--- a/progress_meter.hpp
+++ b/progress_meter.hpp
@@ -63,8 +63,7 @@
 /// cancelled and true otherwise.
 ///
 /// culminate(): Perform postprocessing: call culminate_ui(); then
-/// display a warning if the iteration counter doesn't equal its
-/// maximum, unless the operation was cancelled.
+/// throw if are_postconditions_met() returns false.
 ///
 /// Protected interface--nonvirtual.
 ///
@@ -76,6 +75,11 @@
 ///
 /// Protected interface--virtual.
 ///
+/// dtor: Warn if are_postconditions_met() returns false but the stack
+/// is not being unwound. In that case, an exception should have been
+/// thrown by culminate(), which therefore must not have been called
+/// when it should have been.
+///
 /// do_dawdle(): Implement dawdle().
 ///
 /// progress_message(): Return a string to be displayed when progress
@@ -88,6 +92,13 @@
 /// example, the command-line implementation writes a newline and
 /// flushes its stream.
 ///
+/// Private interface.
+///
+/// are_postconditions_met(): Determine whether postconditions have been
+/// fulfilled, i.e., either
+///  - the iteration counter equals its maximum, or
+///  - the operation was cancelled.
+///
 /// Data members.
 ///
 /// count_: Number of iterations completed so far.
@@ -133,19 +144,10 @@
 /// in the canonical 'for' statement
 ///   for(int i = 0; i < maximum; ++i) {assert(i < maximum);}
 ///
-/// culminate() warns if the iteration counter hasn't been incremented
-/// exactly to its maximum, unless the operation was cancelled. This
-/// is taken as a postcondition of code that uses the progress meter,
-/// so it might seem natural to test it in this class's dtor; however,
-/// the postcondition won't be established if the metered loop is
-/// exited by throwing an exception. Today, std::uncaught_exceptions()
-/// could distinguish that special case, but that was not reliably
-/// possible when this was designed in 2007; there's no need to change
-/// it now. Postcondition failure engenders only a warning because
-/// of other possibilities--e.g., a loop might throw an exception
-/// inside a try-block whose catch-clause doesn't rethrow; perhaps it
-/// would be better to throw an exception instead, depending on the
-/// value of a boolean argument.
+/// It might seem natural to dispense with culminate() and fold its
+/// code into the dtor. However, lmi dtors are designed not to throw,
+/// so the dtor merely warns if culminate() appears not to have been
+/// called when it should have been.
 ///
 /// An argument could be made for making count() public. That's easy
 /// enough to change if wanted, but would promote a usage for which
@@ -177,8 +179,8 @@
 ///     [2005-04-20T01:20:14Z from Greg Chicares]
 ///
 /// Not all data members are actually accessed in any concrete derived
-/// class: max_count_ and title_ are not, but are provided anyway in
-/// case they someday become useful. It might seem desirable to omit
+/// class: for example, title_ is not, but it is provided anyway in
+/// case it someday becomes useful. It might seem desirable to omit
 /// the corresponding create_progress_meter() arguments and set these
 /// members through mutators in this base class after construction
 /// instead of in a derived class's ctor; however, that would not work
@@ -218,7 +220,7 @@ class LMI_SO progress_meter
         ,enum_display_mode
         );
 
-    virtual ~progress_meter() = default;
+    virtual ~progress_meter();
 
     int count() const;
     int max_count() const;
@@ -238,6 +240,8 @@ class LMI_SO progress_meter
     progress_meter(progress_meter const&) = delete;
     progress_meter& operator=(progress_meter const&) = delete;
 
+    bool are_postconditions_met() const;
+
     int               count_;
     int               max_count_;
     std::string       title_;
diff --git a/progress_meter_test.cpp b/progress_meter_test.cpp
index 0a98067..094a8c2 100644
--- a/progress_meter_test.cpp
+++ b/progress_meter_test.cpp
@@ -37,6 +37,7 @@ class progress_meter_test
         test_distinct_metered_operations();
         test_empty_title_and_zero_max_count();
         test_postcondition_failure();
+        test_failure_to_culminate();
         }
 
   private:
@@ -45,6 +46,7 @@ class progress_meter_test
     static void test_distinct_metered_operations();
     static void test_empty_title_and_zero_max_count();
     static void test_postcondition_failure();
+    static void test_failure_to_culminate();
 };
 
 void progress_meter_test::test_normal_usage()
@@ -173,11 +175,11 @@ void progress_meter_test::test_postcondition_failure()
             ,progress_meter::e_unit_test_mode
             )
         );
-    std::cout
-        << "Expect '3 iterations expected, but only 0 completed.':"
-        << std::endl
-        ;
-    meter->culminate();
+    BOOST_TEST_THROW
+        (meter->culminate()
+        ,std::runtime_error
+        ,"3 iterations expected, but only 0 completed."
+        );
 
     for(int i = 0; i < max_count; ++i)
         {
@@ -193,11 +195,11 @@ void progress_meter_test::test_postcondition_failure()
             {
             }
         }
-    std::cout
-        << "Expect '3 iterations expected, but only 2 completed.':"
-        << std::endl
-        ;
-    meter->culminate();
+    BOOST_TEST_THROW
+        (meter->culminate()
+        ,std::runtime_error
+        ,"3 iterations expected, but only 2 completed."
+        );
 
     meter->reflect_progress();
     BOOST_TEST_THROW
@@ -207,6 +209,23 @@ void progress_meter_test::test_postcondition_failure()
         );
 }
 
+void progress_meter_test::test_failure_to_culminate()
+{
+    progress_meter_unit_test_stream().str("");
+    int const max_count = 3;
+    std::shared_ptr<progress_meter> meter
+        (create_progress_meter
+            (max_count
+            ,"Some title"
+            ,progress_meter::e_unit_test_mode
+            )
+        );
+    std::cout
+        << "Expect 'Please report this: culminate() not called.':"
+        << std::endl
+        ;
+}
+
 int test_main(int, char*[])
 {
     progress_meter_test::test();