Merge pull request #18125 from mantidproject/faster_getLine

Avoid extra allocations in Strings::getLine.

Framework/DataHandling/src/PDLoadCharacterizations.cpp

@@ -60,7 +60,7 @@ extra_columns(const std::vector<std::string> &filenames) {
   }
 
   for (std::string line = Strings::getLine(file); !file.eof();
-       line = Strings::getLine(file)) {
+       Strings::getLine(file, line)) {
     boost::smatch result;
     // all instances of table headers
     if (boost::regex_search(line, result, V1_TABLE_REG_EXP)) {
@@ -263,7 +263,7 @@ void PDLoadCharacterizations::readFocusInfo(std::ifstream &file) {
 
   // parse the file
   for (std::string line = Strings::getLine(file); !file.eof();
-       line = Strings::getLine(file)) {
+       Strings::getLine(file, line)) {
     line = Strings::strip(line);
     // skip empty lines and "comments"
     if (line.empty())
@@ -315,9 +315,8 @@ void PDLoadCharacterizations::readCharInfo(std::ifstream &file,
 
   // parse the file
   for (std::string line = Strings::getLine(file); !file.eof();
-       line = Strings::getLine(file)) {
+       Strings::getLine(file, line)) {
     line = Strings::strip(line);
-
     // skip empty lines and "comments"
     if (line.empty())
       continue;
@@ -452,9 +451,8 @@ void PDLoadCharacterizations::readVersion1(const std::string &filename,
 
   // store the names of the columns in order
   std::vector<std::string> columnNames;
-
-  for (std::string line = Strings::getLine(file); !file.eof();
-       line = Strings::getLine(file)) {
+  for (Strings::getLine(file, line); !file.eof();
+       Strings::getLine(file, line)) {
     if (line.empty())
       continue;
     if (line.substr(0, 1) == "#")
@@ -539,7 +537,7 @@ void PDLoadCharacterizations::readExpIni(const std::string &filename,
 
   // parse the file
   for (std::string line = Strings::getLine(file); !file.eof();
-       line = Strings::getLine(file)) {
+       Strings::getLine(file, line)) {
     line = Strings::strip(line);
     // skip empty lines and "comments"
     if (line.empty())

Framework/Kernel/inc/MantidKernel/Strings.h

@@ -101,7 +101,11 @@ MANTID_KERNEL_DLL int isEmpty(const std::string &A);
 /// Determines if a string starts with a #
 MANTID_KERNEL_DLL bool skipLine(const std::string &line);
 /// Get a line and strip comments
-MANTID_KERNEL_DLL std::string getLine(std::istream &fh, const int spc = 256);
+/// Use only for a single call
+MANTID_KERNEL_DLL std::string getLine(std::istream &fh);
+/// Get a line and strip comments
+/// Use within a loop
+MANTID_KERNEL_DLL void getLine(std::istream &fh, std::string &Line);
 /// Peek at a line without extracting it from the stream
 MANTID_KERNEL_DLL std::string peekLine(std::istream &fh);
 /// get a part of a long line

Framework/Kernel/src/Strings.cpp

@@ -273,28 +273,33 @@ std::string removeSpace(const std::string &CLine) {
   return Out;
 }
 
+//------------------------------------------------------------------------------------------------
+/**
+  *  Reads a line from the stream of max length spc.
+  *  Trailing comments are removed. (with # or ! character)
+  *  @param fh :: already open file handle
+  *  @return String read.
+  */
+std::string getLine(std::istream &fh) {
+  std::string line;
+  getLine(fh, line);
+  return line;
+}
+
 //------------------------------------------------------------------------------------------------
 /**
  *  Reads a line from the stream of max length spc.
  *  Trailing comments are removed. (with # or ! character)
  *  @param fh :: already open file handle
- *  @param spc :: max number of characters to read
- *  @return String read.
+ *  @param Line :: string read
  */
-std::string getLine(std::istream &fh, const int spc) {
-  auto ss = new char[spc + 1];
-  std::string Line;
-  if (fh.good()) {
-    fh.getline(ss, spc, '\n');
-    ss[spc] = 0; // incase line failed to read completely
-    Line = ss;
+void getLine(std::istream &fh, std::string &Line) {
+  if (std::getline(fh, Line)) {
     // remove trailing comments
-    std::string::size_type pos = Line.find_first_of("#!");
+    auto pos = Line.find_first_of("#!");
     if (pos != std::string::npos)
       Line.erase(pos);
   }
-  delete[] ss;
-  return Line;
 }
 
 /**

Framework/Kernel/test/StringsTest.h

@@ -419,18 +419,23 @@ public:
   }
 
   void test_toString_vector_of_ints() {
-    std::vector<int> sortedInts;
-    sortedInts.push_back(1);
-    sortedInts.push_back(2);
-    sortedInts.push_back(3);
-    sortedInts.push_back(5);
-    sortedInts.push_back(6);
-    sortedInts.push_back(8);
-
+    std::vector<int> sortedInts{1, 2, 3, 5, 6, 8};
     auto result = toString(sortedInts);
-
     TS_ASSERT_EQUALS(std::string("1-3,5-6,8"), result);
   }
+
+  void test_getLine() {
+    std::istringstream text("blah blah\nfoo bar#comment\n");
+    std::string line = getLine(text);
+    TSM_ASSERT_EQUALS("Strings::getLine failed to read the first line.", line,
+                      "blah blah");
+    getLine(text, line);
+    TSM_ASSERT_EQUALS("Strings::getLine failed to remove comment.", line,
+                      "foo bar");
+    getLine(text, line);
+    TSM_ASSERT_EQUALS("Strings::getLine didn't return empty string after eof.",
+                      line, "");
+  }
 };
 
 #endif // MANTID_SUPPORTTEST_H_