From bda2a87edb3103fbaad0b2b8025f6bada751bada Mon Sep 17 00:00:00 2001
From: Sam Jenkins <s.jenkins@stfc.ac.uk>
Date: Mon, 1 Oct 2018 16:48:19 +0100
Subject: [PATCH] Re #23574 Made Requested changes

---
 Framework/DataHandling/CMakeLists.txt         |  6 +--
 .../{LoadBinStl.h => LoadBinaryStl.h}         | 13 ++++---
 .../src/{LoadBinStl.cpp => LoadBinaryStl.cpp} | 32 ++++++++--------
 .../DataHandling/src/LoadSampleShape.cpp      |  8 ++--
 .../{LoadBinStlTest.h => LoadBinaryStlTest.h} | 38 +++++++++----------
 5 files changed, 51 insertions(+), 46 deletions(-)
 rename Framework/DataHandling/inc/MantidDataHandling/{LoadBinStl.h => LoadBinaryStl.h} (62%)
 rename Framework/DataHandling/src/{LoadBinStl.cpp => LoadBinaryStl.cpp} (77%)
 rename Framework/DataHandling/test/{LoadBinStlTest.h => LoadBinaryStlTest.h} (65%)

diff --git a/Framework/DataHandling/CMakeLists.txt b/Framework/DataHandling/CMakeLists.txt
index 4fcc3c19f51..1fac8ec0ef5 100644
--- a/Framework/DataHandling/CMakeLists.txt
+++ b/Framework/DataHandling/CMakeLists.txt
@@ -39,7 +39,7 @@
 	src/LoadAscii2.cpp
 	src/LoadBBY.cpp
 	src/LoadBankFromDiskTask.cpp
-	src/LoadBinStl.cpp
+	src/LoadBinaryStl.cpp
 	src/LoadCalFile.cpp
 	src/LoadCanSAS1D.cpp
 	src/LoadCanSAS1D2.cpp
@@ -230,7 +230,7 @@ set ( INC_FILES
 	inc/MantidDataHandling/LoadAscii2.h
 	inc/MantidDataHandling/LoadBBY.h
 	inc/MantidDataHandling/LoadBankFromDiskTask.h
-	inc/MantidDataHandling/LoadBinStl.h
+	inc/MantidDataHandling/LoadBinaryStl.h
 	inc/MantidDataHandling/LoadCalFile.h
 	inc/MantidDataHandling/LoadCanSAS1D.h
 	inc/MantidDataHandling/LoadCanSAS1D2.h
@@ -415,7 +415,7 @@ set ( TEST_FILES
 	LoadAscii2Test.h
 	LoadAsciiTest.h
 	LoadBBYTest.h
-	LoadBinStlTest.h
+	LoadBinaryStlTest.h
 	LoadCalFileTest.h
 	LoadCanSAS1dTest.h
 	LoadDaveGrpTest.h
diff --git a/Framework/DataHandling/inc/MantidDataHandling/LoadBinStl.h b/Framework/DataHandling/inc/MantidDataHandling/LoadBinaryStl.h
similarity index 62%
rename from Framework/DataHandling/inc/MantidDataHandling/LoadBinStl.h
rename to Framework/DataHandling/inc/MantidDataHandling/LoadBinaryStl.h
index eb025efff7e..6b5eb3d48f2 100644
--- a/Framework/DataHandling/inc/MantidDataHandling/LoadBinStl.h
+++ b/Framework/DataHandling/inc/MantidDataHandling/LoadBinaryStl.h
@@ -1,5 +1,5 @@
-#ifndef MANTID_DATAHANDLING_LOADBINSTL_H_
-#define MANTID_DATAHANDLING_LOADBINSTL_H_
+#ifndef MANTID_DATAHANDLING_LOADBinaryStl_H_
+#define MANTID_DATAHANDLING_LOADBinaryStl_H_
 #include "MantidGeometry/Objects/MeshObject.h"
 #include "MantidKernel/BinaryStreamReader.h"
 #include <MantidKernel/V3D.h>
@@ -7,9 +7,9 @@
 namespace Mantid {
 namespace DataHandling {
 
-class DLLExport LoadBinStl {
+class DLLExport LoadBinaryStl {
 public:
-  LoadBinStl(std::string filename) : m_filename(filename) {}
+  LoadBinaryStl(std::string filename) : m_filename(filename), M_HEADER_SIZE(80), M_SIZE_OF_TRIANGLE(50), M_NUM_OF_TRIANGLES(4) {}
   std::unique_ptr<Geometry::MeshObject> readStl();
   bool isBinarySTL();
 
@@ -20,10 +20,13 @@ private:
 
   std::vector<uint16_t> m_triangle;
   std::vector<Kernel::V3D> m_verticies;
+  const int M_HEADER_SIZE;
+  const uint32_t M_SIZE_OF_TRIANGLE;
+  const uint32_t M_NUM_OF_TRIANGLES;
 };
 uint16_t addSTLVertex(Kernel::V3D &vertex, std::vector<Kernel::V3D> &vertices);
 bool areEqualVertices(Kernel::V3D const &v1, Kernel::V3D const &v2);
 
 } // namespace DataHandling
 } // namespace Mantid
-#endif /* MANTID_DATAHANDLING_LOADBINSTL_H_ */
\ No newline at end of file
+#endif /* MANTID_DATAHANDLING_LOADBinaryStl_H_ */
\ No newline at end of file
diff --git a/Framework/DataHandling/src/LoadBinStl.cpp b/Framework/DataHandling/src/LoadBinaryStl.cpp
similarity index 77%
rename from Framework/DataHandling/src/LoadBinStl.cpp
rename to Framework/DataHandling/src/LoadBinaryStl.cpp
index cac3e2c2384..5e8d96df669 100644
--- a/Framework/DataHandling/src/LoadBinStl.cpp
+++ b/Framework/DataHandling/src/LoadBinaryStl.cpp
@@ -1,4 +1,4 @@
-#include "MantidDataHandling/LoadBinStl.h"
+#include "MantidDataHandling/LoadBinaryStl.h"
 #include "MantidAPI/FileProperty.h"
 #include "MantidGeometry/Objects/MeshObject.h"
 #include "MantidKernel/BinaryStreamReader.h"
@@ -9,12 +9,12 @@
 namespace Mantid {
 namespace DataHandling {
 
-bool LoadBinStl::isBinarySTL() {
+bool LoadBinaryStl::isBinarySTL() {
   // each triangle is 50 bytes
-  const uint32_t SIZE_OF_TRIANGLE = 50;
+  
   Poco::File stlFile = Poco::File(m_filename);
   auto fileSize = stlFile.getSize();
-  if (fileSize < 84) {
+  if (fileSize < M_HEADER_SIZE + M_NUM_OF_TRIANGLES) {
     // File is smaller than header plus number of triangles, cannot be binary
     // format stl
     return false;
@@ -24,7 +24,7 @@ bool LoadBinStl::isBinarySTL() {
   Kernel::BinaryStreamReader streamReader = Kernel::BinaryStreamReader(myFile);
   numberTrianglesLong = getNumberTriangles(streamReader);
   myFile.close();
-  if (!(fileSize == (84 + (numberTrianglesLong * SIZE_OF_TRIANGLE)))) {
+  if (!(fileSize == (M_HEADER_SIZE + M_NUM_OF_TRIANGLES + (numberTrianglesLong * M_SIZE_OF_TRIANGLE)))) {
     // File is not the Header plus the number of triangles it claims to be long,
     // invalid binary Stl
     return false;
@@ -34,29 +34,31 @@ bool LoadBinStl::isBinarySTL() {
 }
 
 uint32_t
-LoadBinStl::getNumberTriangles(Kernel::BinaryStreamReader streamReader) {
+LoadBinaryStl::getNumberTriangles(Kernel::BinaryStreamReader streamReader) {
   uint32_t numberTrianglesLong;
+  
   // skip header
-  streamReader.moveStreamToPosition(80);
+  streamReader.moveStreamToPosition(M_HEADER_SIZE);
   // Read the number of triangles
   streamReader >> numberTrianglesLong;
   return numberTrianglesLong;
 }
 
-std::unique_ptr<Geometry::MeshObject> LoadBinStl::readStl() {
+std::unique_ptr<Geometry::MeshObject> LoadBinaryStl::readStl() {
   std::ifstream myFile(m_filename.c_str(), std::ios::in | std::ios::binary);
-  uint32_t numberTrianglesLong;
+  const uint32_t SIZE_OF_NORMAL = 12;
+
   Kernel::BinaryStreamReader streamReader = Kernel::BinaryStreamReader(myFile);
-  numberTrianglesLong = getNumberTriangles(streamReader);
-  uint32_t next = 96;
-  const int STEPSIZE = 50;
+  const auto numberTrianglesLong = getNumberTriangles(streamReader);
+  uint32_t nextToRead = M_HEADER_SIZE + M_NUM_OF_TRIANGLES + SIZE_OF_NORMAL;
+
 
   // now read in all the triangles
   for (uint32_t i = 0; i < numberTrianglesLong; i++) {
     // find next triangle, skipping the normal and attribute
-    streamReader.moveStreamToPosition(next);
+    streamReader.moveStreamToPosition(nextToRead);
     readTriangle(streamReader);
-    next += STEPSIZE;
+    nextToRead += M_SIZE_OF_TRIANGLE;
   }
   myFile.close();
   std::unique_ptr<Geometry::MeshObject> retVal =
@@ -66,7 +68,7 @@ std::unique_ptr<Geometry::MeshObject> LoadBinStl::readStl() {
   return retVal;
 }
 
-void LoadBinStl::readTriangle(Kernel::BinaryStreamReader streamReader) {
+void LoadBinaryStl::readTriangle(Kernel::BinaryStreamReader streamReader) {
   // read in the verticies
   for (int i = 0; i < 3; i++) {
     float xVal;
diff --git a/Framework/DataHandling/src/LoadSampleShape.cpp b/Framework/DataHandling/src/LoadSampleShape.cpp
index 31b3cdaa15c..44a5ba2135d 100644
--- a/Framework/DataHandling/src/LoadSampleShape.cpp
+++ b/Framework/DataHandling/src/LoadSampleShape.cpp
@@ -1,5 +1,5 @@
 #include "MantidDataHandling/LoadSampleShape.h"
-#include "MantidDataHandling/LoadBinStl.h"
+#include "MantidDataHandling/LoadBinaryStl.h"
 #include "MantidGeometry/Objects/MeshObject.h"
 
 #include "MantidAPI/FileProperty.h"
@@ -132,10 +132,10 @@ readSTLSolid(std::ifstream &file, std::string &name, std::string filename) {
     boost::trim(line);
     if (line.size() < 5 || line.substr(0, 5) != "solid") {
       // attempt to load stl binary instead
-      std::unique_ptr<LoadBinStl> binaryStlReader =
-          Kernel::make_unique<LoadBinStl>(filename);
+      std::unique_ptr<LoadBinaryStl> binaryStlReader =
+          Kernel::make_unique<LoadBinaryStl>(filename);
       if (binaryStlReader->isBinarySTL()) {
-        return binaryStlReader->LoadBinStl::readStl();
+        return binaryStlReader->LoadBinaryStl::readStl();
       } else {
         throw std::runtime_error("Expected start of solid");
       }
diff --git a/Framework/DataHandling/test/LoadBinStlTest.h b/Framework/DataHandling/test/LoadBinaryStlTest.h
similarity index 65%
rename from Framework/DataHandling/test/LoadBinStlTest.h
rename to Framework/DataHandling/test/LoadBinaryStlTest.h
index 10cb087cc06..386b26bf4e7 100644
--- a/Framework/DataHandling/test/LoadBinStlTest.h
+++ b/Framework/DataHandling/test/LoadBinaryStlTest.h
@@ -1,11 +1,11 @@
-#ifndef LOAD_BINSTL_TEST_H_
-#define LOAD_BINSTL_TEST_H_
+#ifndef LOAD_BinaryStl_TEST_H_
+#define LOAD_BinaryStl_TEST_H_
 
 #include "MantidAPI/AnalysisDataService.h"
 #include "MantidAPI/FileFinder.h"
 #include "MantidAPI/FrameworkManager.h"
 #include "MantidAPI/Sample.h"
-#include "MantidDataHandling/LoadBinStl.h"
+#include "MantidDataHandling/LoadBinaryStl.h"
 #include "MantidDataHandling/LoadInstrument.h"
 #include "MantidGeometry/Objects/MeshObject.h"
 #include "MantidKernel/OptionalBool.h"
@@ -19,16 +19,16 @@ using namespace Mantid::DataHandling;
 using namespace Mantid::DataObjects;
 using namespace Mantid::Geometry;
 
-class LoadBinStlTest : public CxxTest::TestSuite {
+class LoadBinaryStlTest : public CxxTest::TestSuite {
 public:
-  static LoadBinStlTest *createSuite() { return new LoadBinStlTest(); }
-  static void destroySuite(LoadBinStlTest *suite) { delete suite; }
+  static LoadBinaryStlTest *createSuite() { return new LoadBinaryStlTest(); }
+  static void destroySuite(LoadBinaryStlTest *suite) { delete suite; }
 
   void testInit() {}
   void test_cube() {
     std::string path = FileFinder::Instance().getFullPath("cubeBin.stl");
-    std::unique_ptr<LoadBinStl> Loader = std::make_unique<LoadBinStl>(path);
-    auto cube = Loader->readStl();
+    std::unique_ptr<LoadBinaryStl> loader = std::make_unique<LoadBinaryStl>(path);
+    auto cube = loader->readStl();
     TS_ASSERT(cube->hasValidShape());
     TS_ASSERT_EQUALS(cube->numberOfVertices(), 8);
     TS_ASSERT_EQUALS(cube->numberOfTriangles(), 12);
@@ -37,8 +37,8 @@ public:
 
   void test_cylinder() {
     std::string path = FileFinder::Instance().getFullPath("cylinderBin.stl");
-    std::unique_ptr<LoadBinStl> Loader = std::make_unique<LoadBinStl>(path);
-    auto cylinder = Loader->readStl();
+    std::unique_ptr<LoadBinaryStl> loader = std::make_unique<LoadBinaryStl>(path);
+    auto cylinder = loader->readStl();
     TS_ASSERT(cylinder->hasValidShape());
     TS_ASSERT_EQUALS(cylinder->numberOfVertices(), 722);
     TS_ASSERT_EQUALS(cylinder->numberOfTriangles(), 1440);
@@ -47,8 +47,8 @@ public:
 
   void test_tube() {
     std::string path = FileFinder::Instance().getFullPath("tubeBin.stl");
-    std::unique_ptr<LoadBinStl> Loader = std::make_unique<LoadBinStl>(path);
-    auto tube = Loader->readStl();
+    std::unique_ptr<LoadBinaryStl> loader = std::make_unique<LoadBinaryStl>(path);
+    auto tube = loader->readStl();
     TS_ASSERT(tube->hasValidShape());
     TS_ASSERT_EQUALS(tube->numberOfVertices(), 1080);
     TS_ASSERT_EQUALS(tube->numberOfTriangles(), 2160);
@@ -59,23 +59,23 @@ public:
   void test_fail_invalid_vertex() {
     std::string path =
         FileFinder::Instance().getFullPath("invalid_vertexBin.stl");
-    std::unique_ptr<LoadBinStl> Loader = std::make_unique<LoadBinStl>(path);
-    TS_ASSERT(!(Loader->isBinarySTL()));
+    std::unique_ptr<LoadBinaryStl> loader = std::make_unique<LoadBinaryStl>(path);
+    TS_ASSERT(!(loader->isBinarySTL()));
   }
   // check that isBinaryStl returns false if the file contains an incomplete
   // triangle
   void test_fail_invalid_triangle() {
     std::string path =
         FileFinder::Instance().getFullPath("invalid_triangleBin.stl");
-    std::unique_ptr<LoadBinStl> Loader = std::make_unique<LoadBinStl>(path);
-    TS_ASSERT(!(Loader->isBinarySTL()));
+    std::unique_ptr<LoadBinaryStl> loader = std::make_unique<LoadBinaryStl>(path);
+    TS_ASSERT(!(loader->isBinarySTL()));
   }
 
   void test_fail_ascii_stl() {
     std::string path = FileFinder::Instance().getFullPath("cube.stl");
-    std::unique_ptr<LoadBinStl> Loader = std::make_unique<LoadBinStl>(path);
-    TS_ASSERT(!(Loader->isBinarySTL()));
+    std::unique_ptr<LoadBinaryStl> loader = std::make_unique<LoadBinaryStl>(path);
+    TS_ASSERT(!(loader->isBinarySTL()));
   }
 };
 
-#endif /* LOAD_BINSTL_TEST_H_ */
\ No newline at end of file
+#endif /* LOAD_BinaryStl_TEST_H_ */
\ No newline at end of file
-- 
GitLab