[lmi-commits] [lmi] master a202f87 3/3: Serve cached file reads as non-const, rather than const, shared_ptr

[Greg Chicares]

branch: master
commit a202f8754d3b9bf728078203e87c469190753ce4
Author: Gregory W. Chicares <address@hidden>
Commit: Gregory W. Chicares <address@hidden>

    Serve cached file reads as non-const, rather than const, shared_ptr
    
    Remove const_cast from the unit tests. Although it was legal, it could
    too easily have become illegal:
      http://lists.nongnu.org/archive/html/lmi/2016-08/msg00005.html
    
    Add a new accessor
      DBDictionary const& product_database::db() const
    and use it everywhere the shared_ptr member had been used (except in
    unit tests, which aren't production code)--so that the cached database
    is normally accessed through a const API, as is generally desirable:
      http://lists.nongnu.org/archive/html/lmi/2016-08/msg00006.html
    Conveniently, that causes this function:
      product_database::entity_from_key(e_database_key) const
    to choose the const overload of DBDictionary::datum(), when otherwise
    overload resolution would prefer the non-const datum(), which is
    inaccessible in this const member function. Curiously, this accessor
    is private, only because there is no present need for any other class
    to use it.
---
 cache_file_reads.hpp |    8 +++++---
 database.cpp         |   13 +++++++++----
 database.hpp         |    3 ++-
 input_test.cpp       |    2 +-
 premium_tax_test.cpp |    2 +-
 5 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/cache_file_reads.hpp b/cache_file_reads.hpp
index 662ca88..b6b98b4 100644
--- a/cache_file_reads.hpp
+++ b/cache_file_reads.hpp
@@ -44,9 +44,11 @@
 /// For each filename, the cache stores one instance, which is
 /// replaced by reloading the file if its write time has changed.
 ///
-/// Instances are retrieved as shared_ptr<T const> so that they remain
+/// Instances are retrieved as shared_ptr<T> so that they remain
 /// valid even when the file changes. The client is responsible for
-/// updating any stale pointers it holds.
+/// updating any stale pointers it holds. An earlier version returned
+/// shared_ptr<T const> instead, but legitimate non-const use cases
+/// exist, so managing constness is better left to each client.
 ///
 /// Implemented as a simple Meyers singleton, with the expected
 /// dead-reference and threading issues.
@@ -56,7 +58,7 @@ class file_cache
     :private lmi::uncopyable<file_cache<T> >
 {
   public:
-    using retrieved_type = boost::shared_ptr<T const>;
+    using retrieved_type = boost::shared_ptr<T>;
 
     static file_cache<T>& instance()
         {
diff --git a/database.cpp b/database.cpp
index ea52fa8..822c5b0 100644
--- a/database.cpp
+++ b/database.cpp
@@ -179,7 +179,7 @@ namespace
 {
 /// Antediluvian database for static initialization.
 
-boost::shared_ptr<DBDictionary const> antediluvian_db()
+boost::shared_ptr<DBDictionary> antediluvian_db()
 {
     boost::shared_ptr<DBDictionary> z(new DBDictionary);
     z->InitAntediluvian();
@@ -196,7 +196,7 @@ void product_database::initialize(std::string const& 
product_name)
 {
     if(is_antediluvian_fork())
         {
-        static boost::shared_ptr<DBDictionary const> z(antediluvian_db());
+        static boost::shared_ptr<DBDictionary> z(antediluvian_db());
         db_ = z;
         }
     else
@@ -209,11 +209,16 @@ void product_database::initialize(std::string const& 
product_name)
     LMI_ASSERT(0 < length_ && length_ <= methuselah);
 }
 
+DBDictionary const& product_database::db() const
+{
+    LMI_ASSERT(db_);
+    return *db_;
+}
+
 /// Database entity corresponding to the given key.
 
 database_entity const& product_database::entity_from_key(e_database_key k) 
const
 {
-    LMI_ASSERT(db_);
-    return db_->datum(db_name_from_key(k));
+    return db().datum(db_name_from_key(k));
 }
 
diff --git a/database.hpp b/database.hpp
index 24abd64..7d6be79 100644
--- a/database.hpp
+++ b/database.hpp
@@ -78,13 +78,14 @@ class LMI_SO product_database
   private:
     void initialize(std::string const& product_name);
 
+    DBDictionary const& db() const;
     database_entity const& entity_from_key(e_database_key) const;
 
     database_index  index_;
     int             length_;
     int             maturity_age_;
 
-    boost::shared_ptr<DBDictionary const> db_;
+    boost::shared_ptr<DBDictionary> db_;
 };
 
 #endif // database_hpp
diff --git a/input_test.cpp b/input_test.cpp
index 0a92dcb..cb6131f 100644
--- a/input_test.cpp
+++ b/input_test.cpp
@@ -98,7 +98,7 @@ void input_test::test_product_database()
     Input input;
     yare_input yi(input);
     product_database db(yi);
-    DBDictionary& dictionary = const_cast<DBDictionary&>(*db.db_);
+    DBDictionary& dictionary = *db.db_;
 
     std::vector<double> v;
     std::vector<double> w;
diff --git a/premium_tax_test.cpp b/premium_tax_test.cpp
index edd251d..a187045 100644
--- a/premium_tax_test.cpp
+++ b/premium_tax_test.cpp
@@ -106,7 +106,7 @@ void premium_tax_test::test_rates()
     // A uniform but nonzero load would elicit a runtime error,
     // because the tiered load is not zero.
     {
-    DBDictionary& dictionary = const_cast<DBDictionary&>(*db.db_);
+    DBDictionary& dictionary = *db.db_;
 
     database_entity const original = dictionary.datum("PremTaxLoad");
     database_entity const scalar(DB_PremTaxLoad, 0.0000);