[ https://issues.apache.org/jira/browse/GEODE-3420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16246294#comment-16246294 ]
ASF GitHub Bot commented on GEODE-3420: --------------------------------------- dgkimura commented on a change in pull request #143: GEODE-3420: Remove inline header files URL: https://github.com/apache/geode-native/pull/143#discussion_r150057153 ########## File path: cppcache/include/geode/Cacheable.hpp ########## @@ -35,10 +35,10 @@ typedef SerializablePtr CacheablePtr; typedef Serializable Cacheable; template <typename TVALUE> -inline CacheablePtr createValue(const std::shared_ptr<TVALUE>& value); +inline CacheablePtr createValue(const std::shared_ptr<TVALUE>& value) { Review comment: Okay, I gave it a quick try. Region header has some interesting template type resolutions. I can do some type trait magic, but I'm not sure it's worth it. Here's unchanged Region signatures: void put(const CacheableKeyPtr& key, const CacheablePtr& value...) template <class KEYTYPE, class VALUETYPE> void put(const KEYTYPE& key, const VALUETYPE& value... ) If we call `region.put(CacheableString::create("foobar"))` this matches templated put, which uses createValue template to reinterpret CacheableStringPtr into CacheablePtr. Ideally, it seems like instead we'd want the call to match the non-templated signature. In order to do that I can change the signatures to something like follows: template <class KEYTYPE, class VALUETYPE> inline void put( const typename std::enable_if<is_integral_or_char_ptr<KEYTYPE>::value>::type& key = 0, const typename std::enable_if<is_integral_or_char_ptr<VALUETYPE>::value>::type& value = 0, const SerializablePtr& arg = nullptr) { And then we'd have to implement `is_integral_or_char_ptr`, which should be trivial. Is this worth it? Did I miss something? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove all .inl files > --------------------- > > Key: GEODE-3420 > URL: https://issues.apache.org/jira/browse/GEODE-3420 > Project: Geode > Issue Type: Improvement > Components: native client > Reporter: Addison > > Remove all .inc files in favor of a more readable format to describe includes > and templates. -- This message was sent by Atlassian JIRA (v6.4.14#64029)