Refactor RifFormationNamesReader to return std::expected

Replace the QString* errorMessage out-parameter with std::expected<RigFormationNames, QString> in RifFormationNamesReader, and propagate the pattern to RimFormationNames::readFormationNamesFile() which now returns std::expected<void, QString>.

Fatal errors (file cannot be opened, FMU line-parse failure) return std::unexpected. Malformed .lyr lines are still skipped so the rest of the file loads, with warnings logged via RiaLogging instead of collected in the out-parameter. RimFormationNamesCollection::readAllFormationNames() now logs errors it previously discarded.

ApplicationLibCode/Commands/ColorLegendCommands/RicImportColorCategoriesFeature.cpp

@@ -67,13 +67,10 @@ void RicImportColorCategoriesFeature::onActionTriggered( bool isChecked )
     // Remember the path to next time
     app->setLastUsedDialogDirectory( "BINARY_GRID", QFileInfo( fileName ).absolutePath() );
 
-    QString errormessage;
-    auto    formations = RifFormationNamesReader::readFormationNamesFile( fileName, &errormessage );
-    if ( !formations || !errormessage.isEmpty() )
+    auto formations = RifFormationNamesReader::readFormationNamesFile( fileName );
+    if ( !formations )
     {
-        QMessageBox::warning( Riu3DMainWindowTools::mainWindowWidget(),
-                              "Import Formation File Failed",
-                              errormessage.isEmpty() ? "Unknown error reading formation file." : errormessage );
+        QMessageBox::warning( Riu3DMainWindowTools::mainWindowWidget(), "Import Formation File Failed", formations.error() );
         return;
     }
 

ApplicationLibCode/Commands/RicReloadFormationNamesFeature.cpp

@@ -61,11 +61,9 @@ void RicReloadFormationNamesFeature::onActionTriggered( bool isChecked )
     const auto selectedFormationNamesObjs = caf::SelectionManager::instance()->objectsByType<RimFormationNames>();
     for ( RimFormationNames* fnames : selectedFormationNamesObjs )
     {
-        QString errorMessage;
-        fnames->readFormationNamesFile( &errorMessage );
-        if ( !errorMessage.isEmpty() )
+        if ( auto result = fnames->readFormationNamesFile(); !result )
         {
-            RiuMessageDialog::showError( nullptr, "Reload Formation Names", errorMessage );
+            RiuMessageDialog::showError( nullptr, "Reload Formation Names", result.error() );
         }
 
         fnames->updateConnectedViews();

ApplicationLibCode/FileInterface/RifFormationNamesReader.cpp

@@ -18,11 +18,10 @@
 
 #include "RifFormationNamesReader.h"
 
+#include "RiaLogging.h"
 #include "RiaTextStringTools.h"
 #include "RigFormationNames.h"
 
-#include <memory>
-
 #include "cafAssert.h"
 #include "cafPdmUiFilePathEditor.h"
 
@@ -34,35 +33,34 @@
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
-std::unique_ptr<RigFormationNames> RifFormationNamesReader::readFormationNamesFile( const QString& fileName, QString* errorMessage )
+std::expected<RigFormationNames, QString> RifFormationNamesReader::readFormationNamesFile( const QString& fileName )
 {
     QFileInfo fileInfo( fileName );
 
     if ( fileInfo.fileName() == "layer_zone_table.txt" )
     {
-        return RifFormationNamesReader::readFmuFormationNameFile( fileName, errorMessage );
+        return RifFormationNamesReader::readFmuFormationNameFile( fileName );
     }
     else
     {
-        return RifFormationNamesReader::readLyrFormationNameFile( fileName, errorMessage );
+        return RifFormationNamesReader::readLyrFormationNameFile( fileName );
     }
 }
 
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
-std::unique_ptr<RigFormationNames> RifFormationNamesReader::readLyrFormationNameFile( const QString& fileName, QString* errorMessage )
+std::expected<RigFormationNames, QString> RifFormationNamesReader::readLyrFormationNameFile( const QString& fileName )
 {
     QFile dataFile( fileName );
 
     if ( !dataFile.open( QFile::ReadOnly ) )
     {
-        if ( errorMessage ) ( *errorMessage ) += "Could not open file: " + fileName + "\n";
-        return nullptr;
+        return std::unexpected( "Could not open file: " + fileName );
     }
 
-    auto        formationNames = std::make_unique<RigFormationNames>();
-    QTextStream stream( &dataFile );
+    RigFormationNames formationNames;
+    QTextStream       stream( &dataFile );
 
     int lineNumber = 1;
     while ( !stream.atEnd() )
@@ -74,7 +72,7 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readLyrFormationName
         if ( lineSegs.size() == 1 ) continue; // No name present. Comment line ?
         if ( lineSegs.size() == 2 )
         {
-            if ( errorMessage ) ( *errorMessage ) += "Missing quote on line : " + QString::number( lineNumber ) + "\n";
+            RiaLogging::warning( ( fileName + ": missing quote on line " + QString::number( lineNumber ) ).toStdString() );
             continue; // One quote present
         }
 
@@ -103,7 +101,7 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readLyrFormationName
 
                 if ( !( isNumber2 && isNumber1 ) )
                 {
-                    if ( errorMessage ) ( *errorMessage ) += "Format error on line: " + QString::number( lineNumber ) + "\n";
+                    RiaLogging::warning( ( fileName + ": format error on line " + QString::number( lineNumber ) ).toStdString() );
                     continue;
                 }
 
@@ -116,11 +114,11 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readLyrFormationName
                     cvf::Color3f formationColor;
 
                     convertStringToColor( colorWord, &formationColor );
-                    formationNames->appendFormationRange( formationName, formationColor, startK - 1, endK - 1 );
+                    formationNames.appendFormationRange( formationName, formationColor, startK - 1, endK - 1 );
                 }
                 else // no color present
                 {
-                    formationNames->appendFormationRange( formationName, startK - 1, endK - 1 );
+                    formationNames.appendFormationRange( formationName, startK - 1, endK - 1 );
                 }
             }
             else if ( numberWords.size() == 1 )
@@ -130,7 +128,7 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readLyrFormationName
 
                 if ( !isNumber1 )
                 {
-                    if ( errorMessage ) ( *errorMessage ) += "Format error on line: " + QString::number( lineNumber ) + "\n";
+                    RiaLogging::warning( ( fileName + ": format error on line " + QString::number( lineNumber ) ).toStdString() );
                     continue;
                 }
 
@@ -139,16 +137,16 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readLyrFormationName
                     cvf::Color3f formationColor;
 
                     convertStringToColor( colorWord, &formationColor );
-                    formationNames->appendFormationRangeHeight( formationName, formationColor, kLayerCount );
+                    formationNames.appendFormationRangeHeight( formationName, formationColor, kLayerCount );
                 }
                 else // no color present
                 {
-                    formationNames->appendFormationRangeHeight( formationName, kLayerCount );
+                    formationNames.appendFormationRangeHeight( formationName, kLayerCount );
                 }
             }
             else
             {
-                if ( errorMessage ) ( *errorMessage ) += "Format error on line: " + QString::number( lineNumber ) + "\n";
+                RiaLogging::warning( ( fileName + ": format error on line " + QString::number( lineNumber ) ).toStdString() );
             }
         }
 
@@ -161,18 +159,17 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readLyrFormationName
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
-std::unique_ptr<RigFormationNames> RifFormationNamesReader::readFmuFormationNameFile( const QString& fileName, QString* errorMessage )
+std::expected<RigFormationNames, QString> RifFormationNamesReader::readFmuFormationNameFile( const QString& fileName )
 {
     QFile dataFile( fileName );
 
     if ( !dataFile.open( QFile::ReadOnly ) )
     {
-        if ( errorMessage ) ( *errorMessage ) += "Could not open file: " + fileName + "\n";
-        return nullptr;
+        return std::unexpected( "Could not open file: " + fileName );
     }
 
-    auto        formationNames = std::make_unique<RigFormationNames>();
-    QTextStream stream( &dataFile );
+    RigFormationNames formationNames;
+    QTextStream       stream( &dataFile );
 
     int lineNumber = 1;
 
@@ -188,7 +185,7 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readFmuFormationName
             // Make sure we append the last formation
             if ( !currentFormationName.isEmpty() )
             {
-                formationNames->appendFormationRange( currentFormationName, startK - 1, endK - 1 );
+                formationNames.appendFormationRange( currentFormationName, startK - 1, endK - 1 );
             }
             break;
         }
@@ -203,8 +200,7 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readFmuFormationName
 
             if ( lineStream.status() != QTextStream::Ok )
             {
-                *errorMessage = QString( "Failed to parse line %1 of '%2'" ).arg( lineNumber ).arg( fileName );
-                return formationNames;
+                return std::unexpected( QString( "Failed to parse line %1 of '%2'" ).arg( lineNumber ).arg( fileName ) );
             }
 
             if ( formationName != currentFormationName )
@@ -212,7 +208,7 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readFmuFormationName
                 // Append previous formation
                 if ( !currentFormationName.isEmpty() )
                 {
-                    formationNames->appendFormationRange( currentFormationName, startK - 1, endK - 1 );
+                    formationNames.appendFormationRange( currentFormationName, startK - 1, endK - 1 );
                 }
 
                 // Start new formation
@@ -230,7 +226,7 @@ std::unique_ptr<RigFormationNames> RifFormationNamesReader::readFmuFormationName
     // Append previous formation at the end of the stream
     if ( !currentFormationName.isEmpty() )
     {
-        formationNames->appendFormationRange( currentFormationName, startK - 1, endK - 1 );
+        formationNames.appendFormationRange( currentFormationName, startK - 1, endK - 1 );
     }
 
     return formationNames;

ApplicationLibCode/FileInterface/RifFormationNamesReader.h

@@ -20,10 +20,11 @@
 #include "cafPdmField.h"
 #include "cafPdmObject.h"
 
-#include <memory>
+#include "RigFormationNames.h"
 
-class RigFormationNames;
-class QString;
+#include <expected>
+
+#include <QString>
 
 namespace cvf
 {
@@ -36,11 +37,11 @@ class Color3f;
 class RifFormationNamesReader
 {
 public:
-    [[nodiscard]] static std::unique_ptr<RigFormationNames> readFormationNamesFile( const QString& fileName, QString* errorMessage );
+    [[nodiscard]] static std::expected<RigFormationNames, QString> readFormationNamesFile( const QString& fileName );
 
 private:
-    static std::unique_ptr<RigFormationNames> readLyrFormationNameFile( const QString& fileName, QString* errorMessage );
-    static std::unique_ptr<RigFormationNames> readFmuFormationNameFile( const QString& fileName, QString* errorMessage );
+    static std::expected<RigFormationNames, QString> readLyrFormationNameFile( const QString& fileName );
+    static std::expected<RigFormationNames, QString> readFmuFormationNameFile( const QString& fileName );
 
     static bool convertStringToColor( const QString& word, cvf::Color3f* color );
 };

ApplicationLibCode/ProjectDataModel/Formations/RimFormationNames.cpp

@@ -67,11 +67,9 @@ void RimFormationNames::fieldChangedByUi( const caf::PdmFieldHandle* changedFiel
     if ( &m_formationNamesFileName == changedField )
     {
         updateUiTreeName();
-        QString errorMessage;
-        readFormationNamesFile( &errorMessage );
-        if ( !errorMessage.isEmpty() )
+        if ( auto result = readFormationNamesFile(); !result )
         {
-            RiuMessageDialog::showError( nullptr, "Formation Names", errorMessage );
+            RiuMessageDialog::showError( nullptr, "Formation Names", result.error() );
         }
         updateConnectedViews();
     }
@@ -150,9 +148,17 @@ void RimFormationNames::updateConnectedViews()
 //--------------------------------------------------------------------------------------------------
 ///
 //--------------------------------------------------------------------------------------------------
-void RimFormationNames::readFormationNamesFile( QString* errorMessage )
+std::expected<void, QString> RimFormationNames::readFormationNamesFile()
 {
-    m_formationNamesData = RifFormationNamesReader::readFormationNamesFile( m_formationNamesFileName().path(), errorMessage );
+    auto result = RifFormationNamesReader::readFormationNamesFile( m_formationNamesFileName().path() );
+    if ( !result )
+    {
+        m_formationNamesData.reset();
+        return std::unexpected( result.error() );
+    }
+
+    m_formationNamesData = std::make_unique<RigFormationNames>( std::move( *result ) );
+    return {};
 }
 
 //--------------------------------------------------------------------------------------------------

ApplicationLibCode/ProjectDataModel/Formations/RimFormationNames.h

@@ -20,6 +20,7 @@
 #include "cafPdmField.h"
 #include "cafPdmObject.h"
 
+#include <expected>
 #include <memory>
 
 class RigFormationNames;
@@ -45,7 +46,7 @@ public:
 
     void updateConnectedViews();
 
-    void readFormationNamesFile( QString* errorMessage );
+    std::expected<void, QString> readFormationNamesFile();
 
     static QString layerZoneTableFileName();
 

ApplicationLibCode/ProjectDataModel/Formations/RimFormationNamesCollection.cpp

@@ -46,7 +46,10 @@ void RimFormationNamesCollection::readAllFormationNames()
 {
     for ( RimFormationNames* fmNames : m_formationNamesList )
     {
-        fmNames->readFormationNamesFile( nullptr );
+        if ( auto result = fmNames->readFormationNamesFile(); !result )
+        {
+            RiaLogging::error( result.error().toStdString() );
+        }
         RimProject::current()->colorLegendCollection->createColorLegendFromFormationNames( fmNames );
     }
 }
@@ -90,12 +93,9 @@ std::vector<RimFormationNames*> RimFormationNamesCollection::importFiles( const
 
     for ( RimFormationNames* fmNames : formNamesObjsToReload )
     {
-        QString errormessage;
-
-        fmNames->readFormationNamesFile( &errormessage );
-        if ( !errormessage.isEmpty() )
+        if ( auto result = fmNames->readFormationNamesFile(); !result )
         {
-            totalErrorMessage += "\nError in: " + fmNames->fileName() + "\n\t" + errormessage;
+            totalErrorMessage += "\nError in: " + fmNames->fileName() + "\n\t" + result.error();
         }
     }
 

ApplicationLibCode/UnitTests/RifFormationNamesReader-Test.cpp

@@ -18,10 +18,8 @@ TEST( RifFormationNamesReader, ReadLYRFileWithoutColor )
     const QString filePath = baseFolder.absoluteFilePath( filename );
     EXPECT_TRUE( QFile::exists( filePath ) );
 
-    QString errormessage;
-
-    auto fm = RifFormationNamesReader::readFormationNamesFile( filePath, &errormessage );
-    EXPECT_TRUE( errormessage.isEmpty() );
+    auto fm = RifFormationNamesReader::readFormationNamesFile( filePath );
+    ASSERT_TRUE( fm.has_value() );
 
     QString formationName_K1 = fm->formationNameFromKLayerIdx( 0 );
     int     formationIndex   = fm->formationIndexFromKLayerIdx( 1 );
@@ -38,10 +36,8 @@ TEST( RifFormationNamesReader, ReadLYRFileWithColorName )
     const QString filePath = baseFolder.absoluteFilePath( filename );
     EXPECT_TRUE( QFile::exists( filePath ) );
 
-    QString errormessage;
-
-    auto fm = RifFormationNamesReader::readFormationNamesFile( filePath, &errormessage );
-    EXPECT_TRUE( errormessage.isEmpty() );
+    auto fm = RifFormationNamesReader::readFormationNamesFile( filePath );
+    ASSERT_TRUE( fm.has_value() );
 
     QString formationName_K1 = fm->formationNameFromKLayerIdx( 1 );
     int     formationIndex   = fm->formationIndexFromKLayerIdx( 1 );
@@ -65,10 +61,8 @@ TEST( RifFormationNamesReader, ReadLYRFileWithColorHTML )
     const QString filePath = baseFolder.absoluteFilePath( filename );
     EXPECT_TRUE( QFile::exists( filePath ) );
 
-    QString errormessage;
-
-    auto fm = RifFormationNamesReader::readFormationNamesFile( filePath, &errormessage );
-    EXPECT_TRUE( errormessage.isEmpty() );
+    auto fm = RifFormationNamesReader::readFormationNamesFile( filePath );
+    ASSERT_TRUE( fm.has_value() );
 
     QString formationName_K1 = fm->formationNameFromKLayerIdx( 1 );
     int     formationIndex   = fm->formationIndexFromKLayerIdx( 1 );

ApplicationLibCode/UnitTests/RimFormationNames-Test.cpp

@@ -72,9 +72,8 @@ std::unique_ptr<RimFormationNames> readNorneFormationNames()
     auto formationNames = std::make_unique<RimFormationNames>();
     formationNames->setFileName( filePath );
 
-    QString errorMessage;
-    formationNames->readFormationNamesFile( &errorMessage );
-    EXPECT_TRUE( errorMessage.isEmpty() );
+    auto result = formationNames->readFormationNamesFile();
+    EXPECT_TRUE( result.has_value() );
 
     return formationNames;
 }
@@ -106,9 +105,8 @@ TEST( RimFormationNamesTest, CaseDataKeepsFormationNamesAliveAfterReloadAndDelet
     EXPECT_FALSE( namesBeforeReload.empty() );
 
     // Reload replaces the data owned by RimFormationNames
-    QString errorMessage;
-    formationNames->readFormationNamesFile( &errorMessage );
-    EXPECT_TRUE( errorMessage.isEmpty() );
+    auto result = formationNames->readFormationNamesFile();
+    EXPECT_TRUE( result.has_value() );
 
     EXPECT_EQ( namesBeforeReload, joinedFormationNames( mockCase.eclipseCase.p() ) );