How to refactor method using template in C++

Question

I am in need to (or better, I have the chance to) refactor some code in order to make it cleaner.

I would like to use some template, as I think this is a good candidate, in order to reduce the code duplication.

Here is my hpp

class Monetary
{
 public:
  Monetary();
  Monetary(const rapidjson::Value& iMonetary);
  virtual ~Monetary();

  [...cut...]

 private:

  static void initMember(const rapidjson::Value& iMonetary, const char* iName, int& oMember);
  static void initMember(const rapidjson::Value& iMonetary, const char* iName, std::string& oMember);


 private:
  int _amount;
  int _decimal_place;
  std::string _currency;
  std::string _type;
};

And here is the implementation for the the initMember methods:

static void Monetary::initMember(const rapidjson::Value& iMonetary, const char* iName, int& oMember)
{
  rapidjson::Value::ConstMemberIterator aIterator;
  aIterator = iMonetary.FindMember(iName);
  if ( aIterator != iMonetary.MemberEnd() &&
      aIterator->value.IsNumber() )
  {
    oMember = iMonetary[iName].GetInt();
  }
}

static void Monetary::initMember(const rapidjson::Value& iMonetary, const char* iName, std::string& oMember)
{
  rapidjson::Value::ConstMemberIterator aIterator;
  aIterator = iMonetary.FindMember(iName);
  if ( aIterator != iMonetary.MemberEnd() &&
      aIterator->value.IsNumber() )
  {
    oMember = iMonetary[iName].GetString();
  }
}

I was thinking about writing something like

template<typename T>
void Monetary::initMember(const rapidjson::Value& iMonetary, const char* iName, T& oMember)
{
  rapidjson::Value::ConstMemberIterator aIterator;
  aIterator = iMonetary.FindMember(iName);
  if (aIterator == iMonetary.MemberEnd())
  {
    return;
    //throw monetaryException
  }
  assignFromValue(iMonetary[iName], oMember);
}
template<>
void Monetary::assignFromValue<int>(const rapidjson::Value& iValue, int& oMember)
{
  if (!iValue.IsNumber())
  {
    return;
    //throw monetaryException
  }
  oMember = iValue.GetInt();
}
template<>
void Monetary::assignFromValue<std::string>(const rapidjson::Value& iValue, std::string& oMember)
{
  if (!iValue.IsString())
  {
    return;
    //throw monetaryException
  }
  oMember = iValue.GetString();
}

Is there any clewer way to do that ?

score 2 · Accepted Answer · edited May 23 '17 at 11:54

2

My suggestions:

You don't need to make assignFromValue member functions. If you can implement functionality using non-member functions, you should prefer non-member functions. See How Non-Member Functions Improve Encapsulation and How Non-Member Functions Improve Encapsulation.
You don't need to make assignFromValue function templates. They can be simple overloads.

void assignFromValue(const rapidjson::Value& iValue,
                     int& oMember)
{
  if (!iValue.IsNumber())
  {
    return;
    //throw monetaryException
  }
  oMember = iValue.GetInt();
}

void assignFromValue(const rapidjson::Value& iValue,
                     std::string& oMember)
{
  if (!iValue.IsString())
  {
    return;
    //throw monetaryException
  }
  oMember = iValue.GetString();
}

edited May 23 '17 at 11:54

Community

1
1

answered Apr 18 '17 at 15:55

R Sahu

204,454
14
159
270

Sure! I really overlooked it! Template for `assignFromValue` is not needed at all.. – btbbass Apr 19 '17 at 07:34
Tried a bit, I really like the approach. Started refactoring moving methods outside class, it may be good approach (I really hate messy class )! – btbbass Apr 19 '17 at 08:55

Richard Hodges · Answer 2 · 2017-04-18T17:19:00.463

I think I'd do it with a tag-dispatched converter object:

#include <string>
#include <type_traits>
#include <stdexcept>

// simulate your value class
struct Value
{
    bool IsNumber() const;
    bool IsString() const;
    std::string getString() const;
    int getInt() const;
};


// a tag type for easy tag dispatching
template<class Type> struct tag {};


// a converter object contains all rules and conversion sequences
struct ValueConverter
{
    std::string operator()(tag<std::string>, const Value& v) const
    {
        if (not v.IsString()) throw std::invalid_argument("not a string");
        return v.getString();
    }

    int operator()(tag<int>, const Value& v) const
    {
        if (not v.IsNumber()) throw std::invalid_argument("not a number");
        return v.getInt();
    }
};

// write the member once
template<class Target>
void initMember(const Value& iMonetary, const char* iName, Target& oMember)
{
    using target_type = std::decay_t<Target>;
    auto converter = ValueConverter();
    oMember = converter(tag<target_type>(), iMonetary);
}

@RSahu fixed, thank you. I had read `IsNumber()` as the precondition in both code paths. — Richard Hodges, Apr 18 '17 at 17:19

How to refactor method using template in C++

2 Answers2