[Insight-developers] ItkSetStringMacro

[Hans Johnson]

Hello All,

The itkSetStringMacro and itkGetStringMacro assume that the class member is
of type std::string, but the proceed to force the use of char * arguments.
Is there a good reason for this?

I would propose that at a minimum the itkSetStringMacro gets expanded to
generate 2 virtual functions, one exactly like it is, and another that takes
in a "const std::string & _arg".


/** Set character string.  Creates member Set"name"()?
 * (e.g., SetFilename(char *)). The macro assumes that
 * the class member (name) is declared a type std::string. */
#define itkSetStringMacro(name) \
  virtual void Set##name (const char* _arg) \
  { \
    if ( _arg && (_arg == this->m_##name) ) { return;} \
    if (_arg) \
      { \
      this->m_##name = _arg;\
      } \
     else \
      { \
      this->m_##name = ""; \
      } \
    this->Modified(); \
  } \
  virtual void Set##name (const std::string & _arg) \
  { \
    if ( _arg == this->m_##name ) { return;} \
    this->m_##name = _arg;\
    this->Modified(); \
  }


Additionally, I am guessing that the code is correct due to it's use and
passing of all tests, but I don't understand the magic that allows
(_arg  == this->m_##name)
 ^^^^^^   ^^^^^^^^^^^^^^
 char *   std::string
to be correct. Does this get converted to
this->m_##name.operator==(std::string(_argv)) ?   If so, then we could
simply change itkSetStringMacro to the following equivalently efficient code
(at least in my mind):
  virtual void Set##name (const std::string _arg) \
  { \
    if ( _arg == this->m_##name ) { return;} \
    this->m_##name = _arg;\
    this->Modified(); \
  }

I bring this up because the current interface seems clumsy in that you must
often use the .c_str() string member function to assign a local string
variable to a class string member variable.

I'll be happy to make the change, but this seems like there might be hidden
logic in why it was done this way.

Hans