lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] On the morality of using const_cast in unit tests


From: Greg Chicares
Subject: [lmi] On the morality of using const_cast in unit tests
Date: Wed, 3 Aug 2016 16:18:16 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.8.0

Vadim--Is it a code smell to use const_cast in unit tests?

Motivating case: commit fe56ee89d923767307228921f9b3a11c5f6769e1,
abbreviated and rearranged for clarity here:

     database_entity const scalar(DB_PremTaxLoad, 0.0000);
-    DBDictionary::instance().datum("PremTaxLoad") = scalar;
+    DBDictionary& dictionary = const_cast<DBDictionary&>(*db.db_);
+    dictionary.datum("PremTaxLoad") = scalar;

The unit test sets up this "what-if" circumstance in order to validate
class premium_tax's operation.

premium_tax::premium_tax() takes a 'product_database const&' argument.
Class product_database combines two lower-level things that it holds
as private members:
    database_index  index_;
    boost::shared_ptr<DBDictionary const> db_;
which may be thought of as a container of seven-dimensional arrays,
and a particular index into that container. The "what-if" circumstance
above changes the contents of one of the container's arrays.

The original (prior to this month) cache held only one such container,
and was non-const. The new caching implementation holds any number of
such containers, and its API provides only const access (i.e., through
a shared_ptr<T const>). This const API seemed like a good idea because
production code should never change any cached database; but unit tests
do have good reason to change it.

It is legal to use const_cast in the unit tests because the array was
born as a non-const object:
    // Construct before inserting because ctor might throw.
    retrieved_type value(new T(filename));
However, if we ever change that to make it const from birth, it will
undetectably become illegal. And arguably it's immoral anyway.

It might be better to use shared_ptr<T> instead, both in the caching
templates, and in class product_database (which holds it as a private
member and dereferences it only in a single const function anyway).
Then friendship lets unit tests access the private (non-const) member
without const_cast. What's your opinion?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]