[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.
- [lmi] Request for code review,
Greg Chicares <=