lmi
[Top][All Lists]
Advanced

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

[lmi] Request for code review


From: Greg Chicares
Subject: [lmi] Request for code review
Date: Mon, 23 Jan 2017 20:11:00 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0

Two (tested) changes here. The "auto" change seems obvious. The other
is Meyers's "Avoid Duplication in const and Non-const Member Function"
technique. It's already used elsewhere in this file, and I kind of
like it. I imagine it could be written more simply with C++11 [0]. But
first, is it still a good idea?

http://stackoverflow.com/questions/123758/how-do-i-remove-code-duplication-between-similar-const-and-non-const-member-func/123995#123995

| Actually my thinking on this has changed over time: the template
| solution is the only way to avoid duplication and get compiler-checked
| const-correctness, so personally I would no longer use a const_cast in
| order to avoid duplicating code, I'd choose between putting the duped
| code into a function template or else leaving it duped. – Steve Jessop

---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
diff --git a/any_member.hpp b/any_member.hpp
index 75b600b..e7f8abd 100644
--- a/any_member.hpp
+++ b/any_member.hpp
@@ -587,12 +587,17 @@ any_member<ClassType>& 
MemberSymbolTable<ClassType>::operator[]
     (std::string const& s
     )
 {
-    typename member_map_type::iterator i = map_.find(s);
+    return const_cast<any_member<ClassType>&>
+        (static_cast<MemberSymbolTable<ClassType>const&>(*this).operator[](s)
+        );
+#if 0
+    auto i = map_.find(s);
     if(map_.end() == i)
         {
         complain_that_no_such_member_is_ascribed(s);
         }
     return i->second;
+#endif // 0
 }
 
 template<typename ClassType>
@@ -600,7 +605,7 @@ any_member<ClassType> const& 
MemberSymbolTable<ClassType>::operator[]
     (std::string const& s
     ) const
 {
-    typename member_map_type::const_iterator i = map_.find(s);
+    auto i = map_.find(s);
     if(map_.end() == i)
         {
         complain_that_no_such_member_is_ascribed(s);
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------

---------

[0] "I imagine it could be written more simply with C++11"

Here's an example that passes the 'any_member' unit test--add these:

template<typename T>
inline typename std::add_const<T>::type& constify(T& t)
{
    return t;
}

template<typename T>
inline typename std::remove_const<T>::type& unconstify(T& t)
{
    return const_cast<typename std::remove_const<T>::type&>(t);
}

and then change the implementation of the non-const operator[] to this:

    return unconstify(constify(*this).operator[](s));

Probably those [un]constify implementations are woeful in more ways
than I can imagine, but if they could be done correctly, then the
idiom to avoid duplication could become simple and robust. If this
is a worthwhile idea, then the central idea
  unconstify(constify(*this).Function(Argument))
could presumably be written more concisely by deducing the types of
Function and Argument.



reply via email to

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