Re #19961 cleaned up logic in getFacilityFilename

Framework/Kernel/src/ConfigService.cpp

@@ -1776,13 +1776,15 @@ std::string ConfigServiceImpl::getFacilityFilename(const std::string &fName) {
       this->getString("UpdateInstrumentDefinitions.OnStartup");
 
   auto instrDir = directoryNames.begin();
-  if (updateInstrStr == "1" || updateInstrStr == "on" ||
-      updateInstrStr == "On") {
-    // do nothing
-  } else {
-    instrDir++; // advance to after the first value
+
+  // If we are not updating the instrument definitions
+  // update the iterator, this means we will skip the folder in HOME and
+  // look in the instrument folder in mantid install directory or mantid source code directory
+  if (updateInstrStr != "1" || updateInstrStr != "on" || updateInstrStr != "On") {
+    instrDir++;
   }
 
+  // look through all the possible files
   for (; instrDir != directoryNames.end(); ++instrDir) {
     std::string filename = (*instrDir) + "Facilities.xml";
 
@@ -1800,49 +1802,51 @@ std::string ConfigServiceImpl::getFacilityFilename(const std::string &fName) {
 
 /**
  * Load facility information from instrumentDir/Facilities.xml file if fName
- * parameter is not set
+ * parameter is not set.
+ *
+ * If any of the steps fail, we cannot sensibly recover, because the
+ * Facilities.xml file is missing or corrupted.
+ *
  * @param fName :: An alternative file name for loading facilities information.
+ * @throws std::runtime_error :: If the file is not found or fails to parse
  */
 void ConfigServiceImpl::updateFacilities(const std::string &fName) {
   clearFacilities();
 
-  try {
-    std::string fileName = getFacilityFilename(fName);
-
-    // Set up the DOM parser and parse xml file
-    Poco::AutoPtr<Poco::XML::Document> pDoc;
-    try {
-      Poco::XML::DOMParser pParser;
-      pDoc = pParser.parse(fileName);
-    } catch (...) {
-      throw Kernel::Exception::FileError("Unable to parse file:", fileName);
-    }
+  // Try to find the file. If it does not exist we will crash, and cannot read
+  // the Facilities file
+  std::string fileName = getFacilityFilename(fName);
 
-    // Get pointer to root element
-    Poco::XML::Element *pRootElem = pDoc->documentElement();
-    if (!pRootElem->hasChildNodes()) {
-      throw std::runtime_error("No root element in Facilities.xml file");
-    }
+  // Set up the DOM parser and parse xml file
+  Poco::AutoPtr<Poco::XML::Document> pDoc;
+  try {
+    Poco::XML::DOMParser pParser;
+    pDoc = pParser.parse(fileName);
+  } catch (...) {
+    throw Kernel::Exception::FileError("Unable to parse file:", fileName);
+  }
 
-    Poco::AutoPtr<Poco::XML::NodeList> pNL_facility =
-        pRootElem->getElementsByTagName("facility");
-    unsigned long n = pNL_facility->length();
+  // Get pointer to root element
+  Poco::XML::Element *pRootElem = pDoc->documentElement();
+  if (!pRootElem->hasChildNodes()) {
+    throw std::runtime_error("No root element in Facilities.xml file");
+  }
 
-    for (unsigned long i = 0; i < n; ++i) {
-      Poco::XML::Element *elem =
-          dynamic_cast<Poco::XML::Element *>(pNL_facility->item(i));
-      if (elem) {
-        m_facilities.push_back(new FacilityInfo(elem));
-      }
-    }
+  Poco::AutoPtr<Poco::XML::NodeList> pNL_facility =
+      pRootElem->getElementsByTagName("facility");
+  size_t n = pNL_facility->length();
 
-    if (m_facilities.empty()) {
-      throw std::runtime_error("The facility definition file " + fileName +
-                               " defines no facilities");
+  for (size_t i = 0; i < n; ++i) {
+    Poco::XML::Element *elem =
+        dynamic_cast<Poco::XML::Element *>(pNL_facility->item(i));
+    if (elem) {
+      m_facilities.push_back(new FacilityInfo(elem));
     }
+  }
 
-  } catch (std::exception &e) {
-    g_log.error(e.what());
+  if (m_facilities.empty()) {
+    throw std::runtime_error("The facility definition file " + fileName +
+                             " defines no facilities");
   }
 }