Re #11508 Addressing the reviewer comments

1) Tooltip name changed to short description

2) Why simply not use m_description = source?
because for strings m_description = source compiles into something like
if !(&rhs == this)
   lhs.assign(rhs)

3) The names of these methods don't properly reflect what they do. For example, Component::getDescription would imply that the description is about the component, not the parameter...

The name means exactly this. Component::getDescription provides the description for a parametrized component. By current agreement (at least enforced by unit tests), a parametrized component would have a parameter with the same name. If it does not, empty description will be returned. If one sets description for a component, a parameter with component name will be created if not already present. I do not need this feature but decided that it would be nice to have as implementation is really trivial, behaves similarly to all other stuff in components and may be useful to somebody.

4) From my perspective, would these not be a better set of methods:  getParameterDescription, getShortParameterDescription,setParameterDescription...

Done. Except I've used getParamDescription etc instead of getParameterDescription -- it is as descriptive as suggested but saves a bit of typing.
setParamDescription is not implemented. It is not entirely trivial and I do not need it so let's leave it to somebody who(if) actually needs it.

5) I think the description would be more sensible as simply the value of the description element rather than an attribute, i.e....

I make it the same as 12 other (parameters? attributes? components?) parts of the Instrument tree are. Though doing it the suggested way would be better, all other parts are implemented this way. Introducing new entities would be a bit too much for such a small change to the instrument tree. But it its really desired we may create a separate ticket to do such change with this and number of other instrument components. I only suspect that the gain is not worth the efforts, though would be nice.

I think the changes made to the ticket address all remarks.

Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h

@@ -72,7 +72,7 @@ class LoadParameterFileTest : public CxxTest::TestSuite
     TS_ASSERT_EQUALS(paramMap.getString(ptrDet.get(), "testString"), "hello world");
 
     param = paramMap.get(ptrDet.get(), "testString");
-    TS_ASSERT_EQUALS(param->getTooltip(),"its test hello word.");
+    TS_ASSERT_EQUALS(param->getShortDescription(),"its test hello word.");
     TS_ASSERT_EQUALS(param->getDescription(),"its test hello word.");
     TS_ASSERT_EQUALS(paramMap.getDescription("pixel","testString"),"its test hello word.");
 
@@ -158,7 +158,7 @@ class LoadParameterFileTest : public CxxTest::TestSuite
     TS_ASSERT_EQUALS( paramMap.getString(ptrDet.get(), "testString"), "goodbye world");
 
     param = paramMap.get(ptrDet.get(), "testString");
-    TS_ASSERT_EQUALS(param->getTooltip(),"its test goodbye world.");
+    TS_ASSERT_EQUALS(param->getShortDescription(),"its test goodbye world.");
     TS_ASSERT_EQUALS(param->getDescription(),"its test goodbye world.");
     TS_ASSERT_EQUALS(paramMap.getDescription("pixel","testString"),"its test goodbye world.");
 

Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/Component.h

@@ -226,14 +226,14 @@ class MANTID_GEOMETRY_DLL Component : public virtual IComponent {
   std::string getDescription() const;
 
   /** Get description of a parameter attached to this component  */
-  std::string getParDescription(const std::string &pname,
+  std::string getParamDescription(const std::string &pname,
                                bool recursive = true) const;
 
-  /** Get a parameter's tooltip (short description) */
-  std::string getParTooltip(const std::string &pname,
+  /** Get a component's parameter short description */
+  std::string getParamShortDescription(const std::string &pname,
                                bool recursive = true) const;
-  /** Get a parameter's tooltip (short description) -- no recursive search within children*/
-  std::string getTooltip() const;
+  /** Get a components's short description*/
+  std::string getShortDescription() const;
   /**Set components description. Works for parameterized components only */
   void setDescription(const std::string &descr);
   /**

Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/Parameter.h

@@ -96,8 +96,8 @@ class MANTID_GEOMETRY_DLL Parameter {
   {m_description.assign(source);}
   /// get description
   virtual const std::string & getDescription()const{return m_description;}
-  /// get short description (tooltip)
-  virtual std::string getTooltip()const;
+  /// get short description 
+  virtual std::string getShortDescription()const;
   /// Equality operator
   bool operator==(const Parameter &rhs) const {
     if (this->name() == rhs.name() && this->type() == rhs.type() &&

Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h

@@ -252,7 +252,7 @@ class MANTID_GEOMETRY_DLL ParameterMap {
   const std::string getDescription(const std::string &compName,
                                 const std::string &name) const;
   /** Get the component tooltip by name */
-  const std::string getTooltip(const std::string &compName,
+  const std::string getShortDescription(const std::string &compName,
                                 const std::string &name) const;
 
   /// Return the value of a parameter as a string

Code/Mantid/Framework/Geometry/src/Instrument/Component.cpp

@@ -565,7 +565,7 @@ namespace Mantid {
     * or empty string if not.
     */
 
-    std::string Component::getParDescription(const std::string &pname,
+    std::string Component::getParamDescription(const std::string &pname,
       bool recursive) const {
         if (!m_map){ // no description for non-parameterized components
           return std::string("");
@@ -584,12 +584,12 @@ namespace Mantid {
     /** Get this parameter's description -- no recursive search within children*/
     std::string Component::getDescription() const{
       auto name = this->getName();
-      return this->getParDescription(name,false);
+      return this->getParamDescription(name,false);
 
     }
 
     /**
-    * Get a parameter's tooltip (short description)
+    * Get a parameter's short description
     * Only parameterized component can have description
     *
     * @param pname ::     The name of the parameter
@@ -598,7 +598,7 @@ namespace Mantid {
     * @returns std::string describing parameter if such description is defined
     * or empty string if not.
     */
-    std::string Component::getParTooltip(const std::string &pname,
+    std::string Component::getParamShortDescription(const std::string &pname,
       bool recursive) const {
         if (!m_map){ // no tooltips for non-parameterized components
           return std::string("");
@@ -610,15 +610,15 @@ namespace Mantid {
           param = m_map->get(this, pname);
         }
         if (param)
-          return param->getTooltip();
+          return param->getShortDescription();
         else
           return std::string("");
     }
 
     /** Get a parameter's tooltip (short description) -- no recursive search within children*/
-    std::string Component::getTooltip() const{
+    std::string Component::getShortDescription() const{
       auto name = this->getName();
-      return this->getParTooltip(name,false);
+      return this->getParamShortDescription(name,false);
     }
     /**Set components description. Works for parameterized components only 
     * @param descr ::  String which describes the component.

Code/Mantid/Framework/Geometry/src/Instrument/Parameter.cpp

@@ -17,7 +17,7 @@ ParameterFactory::FactoryMap ParameterFactory::s_map;
    If no double LF symbol found in the description string, full string
    is returned.
 */
-std::string Parameter::getTooltip()const{
+std::string Parameter::getShortDescription()const{
   size_t pos = m_description.find(".");
   if (pos == std::string::npos){
     return std::string(m_description);

Code/Mantid/Framework/Geometry/src/Instrument/ParameterMap.cpp

@@ -149,14 +149,14 @@ const std::string ParameterMap::getDescription(const std::string &compName,
     }
     return result;
 }
-/** Get the component tooltip by name
+/** Get the component short description by name
    *  @param compName :: The name of the component
    *  @param name :: The name of the parameter
-   *  @return :: the tooltip (short description) for the first parameter 
+   *  @return :: the short description for the first parameter 
    *  found and having non-empty description,
    *  or empty string if no description found.
 */
-const std::string ParameterMap::getTooltip(const std::string &compName,
+const std::string ParameterMap::getShortDescription(const std::string &compName,
                                 const std::string &name) const{
     pmap_cit it;
     std::string result("");
@@ -165,7 +165,7 @@ const std::string ParameterMap::getTooltip(const std::string &compName,
         boost::shared_ptr<Parameter> param =
             get((const IComponent *)(*it).first, name);
         if (param){
-          result = param->getTooltip();
+          result = param->getShortDescription();
           if(!result.empty())
             return result;
         }

Code/Mantid/Framework/Geometry/test/ComponentTest.h

@@ -189,9 +189,9 @@ class ComponentTest : public CxxTest::TestSuite
 
     TS_ASSERT_THROWS_NOTHING(pq.setDescription("This is child description. This is long child description."));
 
-    TS_ASSERT_EQUALS(parent.getTooltip(),"");
-    TS_ASSERT_EQUALS(pq.getTooltip(),"This is child description.");
-    TS_ASSERT_EQUALS(pq.getParTooltip("Child"),"This is child description.");
+    TS_ASSERT_EQUALS(parent.getShortDescription(),"");
+    TS_ASSERT_EQUALS(pq.getShortDescription(),"This is child description.");
+    TS_ASSERT_EQUALS(pq.getParamShortDescription("Child"),"This is child description.");
 
 
     TS_ASSERT_EQUALS(pq.getDescription(),"This is child description. This is long child description.");

Code/Mantid/Framework/Geometry/test/ParameterMapTest.h

@@ -185,16 +185,16 @@ class ParameterMapTest : public CxxTest::TestSuite
     pmapA.addDouble(m_testInstrument.get(), "testDouble", 4.2,&descr);
     auto parD= pmapA.getRecursive(m_testInstrument.get(),"testDouble");
     TS_ASSERT_EQUALS(parD->getDescription(),descr);
-    TS_ASSERT_EQUALS(parD->getTooltip(),"Test description");
+    TS_ASSERT_EQUALS(parD->getShortDescription(),"Test description");
 
     parD->setDescription("Short description. LongDescription.");
     TS_ASSERT_EQUALS(parD->getDescription(),"Short description. LongDescription.");
-    TS_ASSERT_EQUALS(parD->getTooltip(),"Short description.");
+    TS_ASSERT_EQUALS(parD->getShortDescription(),"Short description.");
 
 
     descr = pmapA.getDescription("basic","testDouble");
     TS_ASSERT_EQUALS(descr,"Short description. LongDescription.");
-    descr = pmapA.getTooltip("basic","testDouble");
+    descr = pmapA.getShortDescription("basic","testDouble");
     TS_ASSERT_EQUALS(descr,"Short description.");
 
   }

Code/Mantid/Framework/PythonInterface/mantid/geometry/src/Exports/Component.cpp

@@ -18,8 +18,8 @@ namespace
   BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Component_getStringParameter,Component::getStringParameter,1,2)
   BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Component_getIntParameter,Component::getIntParameter,1,2)
   BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Component_getParameterType,Component::getParameterType,1,2)
-  BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Component_getParTooltip,Component::getParTooltip,1,2)
-  BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Component_getParDescription,Component::getParDescription,1,2)
+  BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Component_getParamShortDescription,Component::getParamShortDescription,1,2)
+  BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Component_getParamDescription,Component::getParamDescription,1,2)
 
 
 }
@@ -36,11 +36,11 @@ void export_Component()
     .def("getStringParameter", &Component::getStringParameter, Component_getStringParameter())
     .def("getIntParameter", &Component::getIntParameter, Component_getIntParameter())
     //
-    .def("getParTooltip", &Component::getParTooltip, Component_getParTooltip())
-    .def("getParDescription", &Component::getParDescription, Component_getParDescription())
-    .def("getTooltip", &Component::getTooltip,"Return the tooltip of current parameterized component")
+    .def("getParamShortDescription", &Component::getParamShortDescription, Component_getParamShortDescription())
+    .def("getParamDescription", &Component::getParamDescription, Component_getParamDescription())
+    .def("getShortDescription", &Component::getShortDescription,"Return the short description of current parameterized component")
     .def("getDescription", &Component::getDescription,"Return the description of current parameterized component")
-    .def("setDescription", &Component::setDescription, "Set component's description, if the component is parameterized component")
+    .def("setDescription", &Component::setDescription, "Set component's description, works only if the component is parameterized component")
 
     // HACK -- python should return parameters regardless of type. this is until rows below do not work
     .def("getParameterType", &Component::getParameterType, Component_getParameterType())