Re #16808 Various mainly minor comments

Framework/DataHandling/inc/MantidDataHandling/LoadMask.h

@@ -83,7 +83,7 @@ private:
                        std::vector<detid_t> &detectors);
 
   void processMaskOnDetectors(const detid2index_map &indexmap, bool tomask,
-                              std::vector<detid_t> singledetids);
+                              const std::vector<detid_t> &singledetids);
   /// Convert spectrum to detector
   void processMaskOnWorkspaceIndex(bool mask,
                                    std::vector<specnum_t> &maskedSpecID,

Framework/DataHandling/src/LoadMask.cpp

@@ -76,8 +76,7 @@ void convertToVector(const std::vector<T> &singles,
   }
 }
 
-/*
-* Parse index range text to singles and pairs
+/** Parse index range text to singles and pairs
 * Example: 3,4,9-10,33
 *
 * @param inputstr -- input string to process in the format as above
@@ -218,7 +217,7 @@ void loadISISMaskFile(const std::string &isisfilename,
   std::vector<Mantid::specnum_t> ranges;
 
   std::ifstream ifs;
-  ifs.open(isisfilename.c_str(), std::ios::in);
+  ifs.open(isisfilename, std::ios::in);
   if (!ifs.is_open()) {
     throw std::invalid_argument("Cannot open ISIS mask file" + isisfilename);
   }
@@ -228,7 +227,7 @@ void loadISISMaskFile(const std::string &isisfilename,
     boost::trim(isisline);
 
     // a. skip empty line
-    if (isisline.size() == 0)
+    if (isisline.empty())
       continue;
 
     // b. skip comment line
@@ -238,7 +237,6 @@ void loadISISMaskFile(const std::string &isisfilename,
     // c. parse
     parseISISStringToVector(isisline, ranges);
   }
-  ifs.close();
 
   // dummy helper vector as ISIS mask is always processed as pairs.
   std::vector<Mantid::specnum_t> dummy;
@@ -309,7 +307,7 @@ void LoadMask::init() {
   declareProperty(
       Kernel::make_unique<WorkspaceProperty<API::MatrixWorkspace>>(
           "RefWorkspace", "", Direction::Input, PropertyMode::Optional),
-      "The name of the workspace with defines instrument and spectra, "
+      "The name of the workspace wich defines instrument and spectra, "
       "used as the source of the spectra-detector map for the mask to load. "
       "The instrument, attached to this workspace has to be the same as the "
       "one specified by 'Instrument' property");
@@ -404,11 +402,10 @@ void LoadMask::initDetectors() {
  *                   in masking
  *   @param tomask:  true to mask, false to unmask
  *   @param singledetids: list of individual det ids to mask
-
  */
-void LoadMask::processMaskOnDetectors(const detid2index_map &indexmap,
-                                      bool tomask,
-                                      std::vector<int32_t> singledetids) {
+void LoadMask::processMaskOnDetectors(
+    const detid2index_map &indexmap, bool tomask,
+    const std::vector<detid_t> &singledetids) {
   // 1. Get index map
   // 2. Mask
   g_log.debug() << "Mask = " << tomask
@@ -433,8 +430,8 @@ void LoadMask::processMaskOnDetectors(const detid2index_map &indexmap,
 /** Extract a component's detectors and return it within detectors array
  *  It is a generalized version of bankToDetectors()
  *
- * @param componentnames -- vector of components names to process
- * @param detectors      -- vector of detectors id, which belongs to components
+ * @param componentnames -- vector of component names to process
+ * @param detectors      -- vector of detector ids, which belongs to components
  *provided as input.
  */
 void LoadMask::componentToDetectors(
@@ -465,8 +462,8 @@ void LoadMask::componentToDetectors(
     g_log.debug() << "Number of Children = " << children.size() << '\n';
 
     size_t numdets(0);
-    detid_t id_min = std::numeric_limits<Mantid::detid_t>::max();
-    detid_t id_max = 0;
+    detid_t id_min(std::numeric_limits<Mantid::detid_t>::max());
+    detid_t id_max(0);
 
     for (const auto &child : children) {
       // c) convert component to detector
@@ -491,7 +488,7 @@ void LoadMask::componentToDetectors(
 
 //----------------------------------------------------------------------------------------------
 /** Convert bank to detectors
-* This routine have never been invoked.
+* This routine has never been used. Dead code.
 * @param   singlebanks -- vector of string containing bank names
 * @param  detectors   -- vector of detector-id-s belonging to these banks
  */
@@ -521,7 +518,7 @@ void LoadMask::bankToDetectors(const std::vector<std::string> &singlebanks,
     // b) set detectors
 
     for (const auto &det : idetectors) {
-      int32_t detid = det->getID();
+      detid_t detid = det->getID();
       detectors.push_back(detid);
     }
     g_log.debug() << "Number of Detectors in Bank  " << singlebank
@@ -682,13 +679,11 @@ void LoadMask::parseXML() {
       } else {
         g_log.error() << "XML File hierarchical (component) error!\n";
       }
-      // g_log.information() << "Component: " << value << '\n';
 
     } else if (pNode->nodeName().compare("ids") == 0) {
       // Node "ids"
       if (ingroup) {
         parseRangeText(value, singleSp, pairSp);
-        // this->parseSpectrumNos(value, m_maskSpecID);
       } else {
         g_log.error() << "XML File (ids) hierarchical error!"
                       << "  Inner Text = " << pNode->innerText() << '\n';
@@ -724,10 +719,10 @@ void LoadMask::parseXML() {
 /* Convert spectra mask into det-id mask using workspace as source of
 *spectra-detector maps
 *
-* @param sourceWS       -- the workspace containing source spectra-detecot map
+* @param sourceWS       -- the workspace containing source spectra-detector map
 *                          to use on masks
 * @param maskedSpecID   -- vector of spectra id to mask
-* @param singleDetIds   -- output vector of detectors id to mask
+* @param singleDetIds   -- output vector of detector ids to mask
 */
 void LoadMask::convertSpMasksToDetIDs(const API::MatrixWorkspace &sourceWS,
                                       const std::vector<int32_t> &maskedSpecID,

Framework/DataHandling/src/MaskDetectors.cpp

@@ -13,7 +13,7 @@
 #include <numeric>
 
 namespace { // declare file scoped function
-/* internal method copies values in specified range from source list to the
+/** internal method copies values in specified range from source list to the
 target list
 @param sourceList :: vector of input values
 @param targetList :: vector of output values
@@ -25,9 +25,9 @@ void constrainIndexInRange(std::vector<size_t> &sourceList,
                            size_t maxIndex) {
   targetList.reserve(sourceList.size());
   std::sort(sourceList.begin(), sourceList.end());
-  for (auto it = sourceList.begin(); it != sourceList.end(); it++) {
-    if (*it >= minIndex && *it <= maxIndex) {
-      targetList.push_back(*it);
+  for (auto memb : sourceList) {
+    if (memb >= minIndex && memb <= maxIndex) {
+      targetList.push_back(memb);
     }
   }
 }
@@ -82,13 +82,13 @@ void MaskDetectors::init() {
   mustBePosInt->setLower(0);
   declareProperty(
       "StartWorkspaceIndex", 0, mustBePosInt,
-      "If other masks fields are provided, its the first index of the "
+      "If other masks fields are provided, it's the first index of the "
       "target workspace to be allowed to be masked from by these masks, "
       "if not, its the first index of the target workspace to mask.\n"
       "Default value is 0 if other masking is present or ignored if not.");
   declareProperty(
       "EndWorkspaceIndex", EMPTY_INT(), mustBePosInt,
-      "If other masks are provided, its the last index of the "
+      "If other masks are provided, it's the last index of the "
       "target workspace allowed to be masked to by these masks, "
       "if not, its the last index of the target workspace to mask.\n"
       "Default is number of histograms in target workspace if other masks are"
@@ -279,13 +279,13 @@ MaskDetectors::getRanges(const MatrixWorkspace_sptr &targWS) {
     return std::tuple<size_t, size_t, bool>(startIndex_l, endIndex_l, true);
   }
 }
-/* Constrain masked indexes by limits, provided as input
+/* Do constrain masked indexes by limits, provided as input
 * @param indexList  :: list of indexes to verify against constrain on input
 *                      and list of constrained indexes on the output.
 * @param startIndex :: minimal index (inclusive) to include in the constrained
-* list.
+*                      list.
 * @param endIndex   :: maximal index (inclusive) to include in the constrained
-* list
+*                      list.
 */
 void MaskDetectors::constrainMaskedIndexes(
     std::vector<size_t> &indexList,
@@ -301,8 +301,7 @@ void MaskDetectors::constrainMaskedIndexes(
 * the mask workspace assumed to be not having masked detectors, but has masked
 * state defined in its spectra
 @param detectorList :: list of masked detectors, appended on output by the
-detectors,
-*                      defined in the mask workspace.
+*                      detectors, defined in the mask workspace.
 @param maskWS       :: shared pointer to workspace containing masks.
 */
 void MaskDetectors::extractMaskedWSDetIDs(
@@ -491,7 +490,6 @@ void MaskDetectors::appendToIndexListFromWS(
   }
   tmp_index.swap(indexList);
 
-  return;
 } // appendToIndexListFromWS
 
 /**
@@ -535,7 +533,6 @@ void MaskDetectors::appendToIndexListFromMaskWS(
     }
   }
   tmp_index.swap(indexList);
-  return;
 } // appendToIndexListFromWS
 
 } // namespace DataHandling

docs/source/algorithms/MaskDetectors-v1.rst

@@ -11,10 +11,10 @@ Introduction
 
 To understand the algorithms options, user should clearly understand the difference between *WorkspaceIndex* 
 -- the numbers, specified in *WorkspaceIndexList* and *StartWorkspacIndex*, *EndWorkspaceIndex* properties,
-the *Spectra ID* -- values of the **SpectraList** property and *Detector ID* -- the numbers to provide for 
+the *Spectra ID* or *Spectra Number* -- values of the **SpectraList** property and *Detector ID* -- the numbers to provide for 
 *DetectorList* property.
 
-The *WorkspaceIndex* is the number a spectra has in a workspace, e.g. ::
+The *WorkspaceIndex* is the number a spectrum has in a workspace, e.g. ::
 
   sp = ws.getSpectrum(0)