Moved Result handling outside of AtomicFile (#941)

- `AtomicFile` now has a `Create` method which returns `Result<AtomicFile>` instead of a public constructor - This result should be checked before using the `AtomicFile` which is in place of the stored `Result` variable - `AtomicFile::commit()` nows also returns a `Result<>` instead a bool - Fixed issue with `ConvertInvalidResult` regarding non default constructible types - `AtomicFile` is now move only Signed-off-by: Jared Duffey <jared.duffey@bluequartz.net>
BlueQuartzSoftware · May 3, 2024 · 9c17e33 · 9c17e33
1 parent 70d5490
commit 9c17e33
Show file tree

Hide file tree

Showing 22 changed files with 311 additions and 316 deletions.
diff --git a/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/ITKImageWriterFilter.cpp b/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/ITKImageWriterFilter.cpp
@@ -50,8 +50,13 @@ bool Is2DFormat(const fs::path& fileName)
 template <typename PixelT, uint32 Dimensions>
 Result<> WriteAsOneFile(itk::Image<PixelT, Dimensions>& image, const fs::path& filePath /*, const IFilter::MessageHandler& messanger*/)
 {
-  AtomicFile atomicFile(filePath.string());
-  auto tempPath = atomicFile.tempFilePath();
+  auto atomicFileResult = AtomicFile::Create(filePath);
+  if(atomicFileResult.invalid())
+  {
+    return ConvertResult(std::move(atomicFileResult));
+  }
+  AtomicFile atomicFile = std::move(atomicFileResult.value());
+  std::string tempPath = atomicFile.tempFilePath().string();
   try
   {
     using ImageType = itk::Image<PixelT, Dimensions>;
@@ -61,17 +66,18 @@ Result<> WriteAsOneFile(itk::Image<PixelT, Dimensions>& image, const fs::path& f
     // messanger(fmt::format("Saving {}", fileName));
 
     writer->SetInput(&image);
-    writer->SetFileName(tempPath.string());
+    writer->SetFileName(tempPath);
     writer->UseCompressionOn();
     writer->Update();
   } catch(const itk::ExceptionObject& err)
   {
     return MakeErrorResult(-21011, fmt::format("ITK exception was thrown while writing output file: {}", err.GetDescription()));
   }
 
-  if(!atomicFile.commit())
+  Result<> commitResult = atomicFile.commit();
+  if(commitResult.invalid())
   {
-    return atomicFile.getResult();
+    return commitResult;
   }
   return {};
 }
@@ -80,42 +86,42 @@ template <typename PixelT, uint32 Dimensions>
 Result<> WriteAs2DStack(itk::Image<PixelT, Dimensions>& image, uint32 z_size, const fs::path& filePath, uint64 indexOffset)
 {
   // Create list of AtomicFiles
-  std::vector<std::unique_ptr<AtomicFile>> atomicFiles = {};
+  std::vector<Result<AtomicFile>> atomicFiles;
+  std::vector<std::string> fileNames;
 
   for(uint64 index = indexOffset; index < (z_size - 1); index++)
   {
-    atomicFiles.emplace_back(
-        std::make_unique<AtomicFile>((fs::absolute(fmt::format("{}/{}{:03d}{}", filePath.parent_path().string(), filePath.stem().string(), index, filePath.extension().string())))));
-  }
-  {
-    std::vector<std::string> fileNames = {};
-    for(const auto& atomicFile : atomicFiles)
+    atomicFiles.push_back(AtomicFile::Create(fs::absolute(fmt::format("{}/{}{:03d}{}", filePath.parent_path().string(), filePath.stem().string(), index, filePath.extension().string()))));
+    auto& atomicFileResult = atomicFiles.back();
+    if(atomicFileResult.invalid())
     {
-      fileNames.emplace_back(atomicFile->tempFilePath().string());
+      return ConvertResult(std::move(atomicFileResult));
     }
+    fileNames.push_back(atomicFileResult.value().tempFilePath().string());
+  }
 
-    // generate all the files in that new directory
-    try
-    {
-      using InputImageType = itk::Image<PixelT, Dimensions>;
-      using OutputImageType = itk::Image<PixelT, Dimensions - 1>;
-      using SeriesWriterType = itk::ImageSeriesWriter<InputImageType, OutputImageType>;
-      auto writer = SeriesWriterType::New();
-      writer->SetInput(&image);
-      writer->SetFileNames(fileNames);
-      writer->UseCompressionOn();
-      writer->Update();
-    } catch(const itk::ExceptionObject& err)
-    {
-      return MakeErrorResult(-21011, fmt::format("ITK exception was thrown while writing output file: {}", err.GetDescription()));
-    }
+  // generate all the files in that new directory
+  try
+  {
+    using InputImageType = itk::Image<PixelT, Dimensions>;
+    using OutputImageType = itk::Image<PixelT, Dimensions - 1>;
+    using SeriesWriterType = itk::ImageSeriesWriter<InputImageType, OutputImageType>;
+    auto writer = SeriesWriterType::New();
+    writer->SetInput(&image);
+    writer->SetFileNames(fileNames);
+    writer->UseCompressionOn();
+    writer->Update();
+  } catch(const itk::ExceptionObject& err)
+  {
+    return MakeErrorResult(-21011, fmt::format("ITK exception was thrown while writing output file: {}", err.GetDescription()));
   }
 
-  for(const auto& atomicFile : atomicFiles)
+  for(auto& atomicFile : atomicFiles)
   {
-    if(!atomicFile->commit())
+    Result<> commitResult = atomicFile.value().commit();
+    if(commitResult.invalid())
     {
-      return atomicFile->getResult();
+      return commitResult;
     }
   }
 

diff --git a/...ntationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ConvertHexGridToSquareGrid.cpp b/...ntationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ConvertHexGridToSquareGrid.cpp
@@ -71,22 +71,20 @@ class Converter
       }
 
       std::istringstream headerStream(origHeader, std::ios_base::in | std::ios_base::binary);
-      m_AtomicFiles.emplace_back(std::make_unique<AtomicFile>((fs::absolute(m_OutputPath) / (m_FilePrefix + inputPath.filename().string()))));
-      auto creationResult = m_AtomicFiles[m_Index]->getResult();
-      if(creationResult.invalid())
+      m_AtomicFiles.push_back(AtomicFile::Create(fs::absolute(m_OutputPath) / (m_FilePrefix + inputPath.filename().string())));
+      if(m_AtomicFiles[m_Index].invalid())
       {
-        return creationResult;
+        return ConvertResult(std::move(m_AtomicFiles[m_Index]));
       }
-      fs::path outPath = m_AtomicFiles[m_Index]->tempFilePath();
+      fs::path outPath = m_AtomicFiles[m_Index].value().tempFilePath();
 
       // Ensure the output path exists by creating it if necessary
       if(!fs::exists(m_OutputPath))
       {
-        auto result = m_AtomicFiles[m_Index]->getResult();
-        if(result.invalid())
+        if(m_AtomicFiles[m_Index].invalid())
         {
           m_Valid = false;
-          return result;
+          return ConvertResult(std::move(m_AtomicFiles[m_Index]));
         }
       }
 
@@ -223,11 +221,12 @@ class Converter
   {
     if(m_Valid)
     {
-      for(const auto& atomicFile : m_AtomicFiles)
+      for(auto& atomicFile : m_AtomicFiles)
       {
-        if(!atomicFile->commit())
+        Result<> commitResult = atomicFile.value().commit();
+        if(commitResult.invalid())
         {
-          return atomicFile->getResult();
+          return commitResult;
         }
       }
     }
@@ -243,7 +242,7 @@ class Converter
   const std::atomic_bool& m_ShouldCancel;
   const fs::path& m_OutputPath;
   const std::string& m_FilePrefix;
-  std::vector<std::unique_ptr<AtomicFile>> m_AtomicFiles = {};
+  std::vector<Result<AtomicFile>> m_AtomicFiles;
   usize m_Index = 0;
   bool m_Valid = true;
 

diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/EbsdToH5Ebsd.cpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/EbsdToH5Ebsd.cpp
@@ -51,12 +51,12 @@ Result<> EbsdToH5Ebsd::operator()()
     }
   }
 
-  AtomicFile atomicFile(absPath.string());
-  auto creationResult = atomicFile.getResult();
-  if(creationResult.invalid())
+  auto atomicFileResult = AtomicFile::Create(absPath);
+  if(atomicFileResult.invalid())
   {
-    return creationResult;
+    return ConvertResult(std::move(atomicFileResult));
   }
+  AtomicFile atomicFile = std::move(atomicFileResult.value());
 
   // Scope file writer in code block to get around file lock on windows (enforce destructor order)
   {
@@ -322,9 +322,10 @@ Result<> EbsdToH5Ebsd::operator()()
     }
   }
 
-  if(!atomicFile.commit())
+  Result<> commitResult = atomicFile.commit();
+  if(commitResult.invalid())
   {
-    return atomicFile.getResult();
+    return commitResult;
   }
   return {};
 }
diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/WriteGBCDGMTFileFilter.cpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/WriteGBCDGMTFileFilter.cpp
@@ -124,12 +124,12 @@ IFilter::PreflightResult WriteGBCDGMTFileFilter::preflightImpl(const DataStructu
 Result<> WriteGBCDGMTFileFilter::executeImpl(DataStructure& dataStructure, const Arguments& filterArgs, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler,
                                              const std::atomic_bool& shouldCancel) const
 {
-  AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
-  auto creationResult = atomicFile.getResult();
-  if(creationResult.invalid())
+  auto atomicFileResult = AtomicFile::Create(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
+  if(atomicFileResult.invalid())
   {
-    return creationResult;
+    return ConvertResult(std::move(atomicFileResult));
   }
+  AtomicFile atomicFile = std::move(atomicFileResult.value());
 
   WriteGBCDGMTFileInputValues inputValues;
 
@@ -142,9 +142,10 @@ Result<> WriteGBCDGMTFileFilter::executeImpl(DataStructure& dataStructure, const
   auto result = WriteGBCDGMTFile(dataStructure, messageHandler, shouldCancel, &inputValues)();
   if(result.valid())
   {
-    if(!atomicFile.commit())
+    Result<> commitResult = atomicFile.commit();
+    if(commitResult.invalid())
     {
-      return atomicFile.getResult();
+      return commitResult;
     }
   }
   return result;

diff --git a/...ugins/OrientationAnalysis/src/OrientationAnalysis/Filters/WriteGBCDTriangleDataFilter.cpp b/...ugins/OrientationAnalysis/src/OrientationAnalysis/Filters/WriteGBCDTriangleDataFilter.cpp
@@ -110,12 +110,12 @@ IFilter::PreflightResult WriteGBCDTriangleDataFilter::preflightImpl(const DataSt
 Result<> WriteGBCDTriangleDataFilter::executeImpl(DataStructure& dataStructure, const Arguments& filterArgs, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler,
                                                   const std::atomic_bool& shouldCancel) const
 {
-  AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
-  auto creationResult = atomicFile.getResult();
-  if(creationResult.invalid())
+  auto atomicFileResult = AtomicFile::Create(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
+  if(atomicFileResult.invalid())
   {
-    return creationResult;
+    return ConvertResult(std::move(atomicFileResult));
   }
+  AtomicFile atomicFile = std::move(atomicFileResult.value());
 
   WriteGBCDTriangleDataInputValues inputValues;
 
@@ -128,9 +128,10 @@ Result<> WriteGBCDTriangleDataFilter::executeImpl(DataStructure& dataStructure,
   auto result = WriteGBCDTriangleData(dataStructure, messageHandler, shouldCancel, &inputValues)();
   if(result.valid())
   {
-    if(!atomicFile.commit())
+    Result<> commitResult = atomicFile.commit();
+    if(commitResult.invalid())
     {
-      return atomicFile.getResult();
+      return commitResult;
     }
   }
   return result;

diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/WriteINLFileFilter.cpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/WriteINLFileFilter.cpp
@@ -111,12 +111,12 @@ IFilter::PreflightResult WriteINLFileFilter::preflightImpl(const DataStructure&
 Result<> WriteINLFileFilter::executeImpl(DataStructure& dataStructure, const Arguments& filterArgs, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler,
                                          const std::atomic_bool& shouldCancel) const
 {
-  AtomicFile atomicFile(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
-  auto creationResult = atomicFile.getResult();
-  if(creationResult.invalid())
+  auto atomicFileResult = AtomicFile::Create(filterArgs.value<FileSystemPathParameter::ValueType>(k_OutputFile_Key));
+  if(atomicFileResult.invalid())
   {
-    return creationResult;
+    return ConvertResult(std::move(atomicFileResult));
   }
+  AtomicFile atomicFile = std::move(atomicFileResult.value());
 
   WriteINLFileInputValues inputValues;
 
@@ -132,9 +132,10 @@ Result<> WriteINLFileFilter::executeImpl(DataStructure& dataStructure, const Arg
   auto result = WriteINLFile(dataStructure, messageHandler, shouldCancel, &inputValues)();
   if(result.valid())
   {
-    if(!atomicFile.commit())
+    Result<> commitResult = atomicFile.commit();
+    if(commitResult.invalid())
     {
-      return atomicFile.getResult();
+      return commitResult;
     }
   }
   return result;