Skip to content

Refactor well log track#628

Open
magnesj wants to merge 5 commits intodevfrom
refactor-well-log-track
Open

Refactor well log track#628
magnesj wants to merge 5 commits intodevfrom
refactor-well-log-track

Conversation

@magnesj
Copy link
Owner

@magnesj magnesj commented Jan 26, 2026

PR Type

Enhancement


Description

  • Refactored RimWellLogTrack into modular helper classes to improve code organization and maintainability

  • Extracted property axis management into RimWellLogTrackPropertyAxis (371 lines) with zoom range calculation, tick interval management, and logarithmic scale handling

  • Created RimWellLogTrackStackedCurves helper class for managing stacked curve data, visibility, phase-based coloring, and z-order positioning

  • Implemented RimWellLogTrackWellPathComponents (222 lines) for well path attributes, completions visualization, and depth range calculations

  • Added RimWellLogTrackRegionAnnotations for formation names, result properties, water/rock region visualization, and well bore stability plot annotations

  • Introduced RimWellLogTrackTools namespace with static utility functions for curve sampling, formation vector retrieval, region name finding, and ASCII export formatting

  • Replaced inline implementations in main class with delegating calls to helper classes

  • Updated CMake configuration to include five new header and source files


Diagram Walkthrough

flowchart LR
  RimWellLogTrack["RimWellLogTrack<br/>Main Class"]
  PropertyAxis["RimWellLogTrackPropertyAxis<br/>Axis Management"]
  StackedCurves["RimWellLogTrackStackedCurves<br/>Curve Stacking"]
  WellPath["RimWellLogTrackWellPathComponents<br/>Path Visualization"]
  RegionAnnotations["RimWellLogTrackRegionAnnotations<br/>Region Annotations"]
  Tools["RimWellLogTrackTools<br/>Utility Functions"]
  
  RimWellLogTrack -- "delegates to" --> PropertyAxis
  RimWellLogTrack -- "delegates to" --> StackedCurves
  RimWellLogTrack -- "delegates to" --> WellPath
  RimWellLogTrack -- "delegates to" --> RegionAnnotations
  PropertyAxis -- "uses" --> Tools
  StackedCurves -- "uses" --> Tools
  WellPath -- "uses" --> Tools
  RegionAnnotations -- "uses" --> Tools
Loading

File Walkthrough

Relevant files
Refactoring
2 files
RimWellLogTrack.cpp
Extract well log track functionality into helper classes 

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrack.cpp

  • Refactored large methods into separate helper classes
    (RimWellLogTrackPropertyAxis, RimWellLogTrackStackedCurves,
    RimWellLogTrackWellPathComponents, RimWellLogTrackRegionAnnotations)
  • Extracted utility functions to RimWellLogTrackTools namespace for
    better code organization
  • Added unique_ptr members for property axis, stacked curves, well path
    components, and region annotations
  • Replaced inline implementations with delegating calls to helper
    classes and tools
  • Added public accessor methods for private fields to support helper
    classes
+249/-1305
RimWellLogTrack.h
Refactored well log track with helper classes                       

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrack.h

  • Refactored to use new helper classes for property axis, stacked
    curves, region annotations, and well path components
  • Added forward declarations for new helper classes
  • Moved accessor methods to public interface for helper class access
  • Replaced direct member variables with unique_ptr instances to helper
    classes
  • Added new accessor methods for formation source, color shading, and
    property axis configuration
+52/-19 
Enhancement
10 files
RimWellLogTrackRegionAnnotations.cpp
New region annotations helper class implementation             

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackRegionAnnotations.cpp

  • New file implementing region annotation management for well log tracks
  • Handles formation names, result property names, and curve data region
    annotations
  • Manages water and rock region visualization with proper depth unit
    conversion
  • Supports well bore stability plot annotations with multiple data
    sources
+585/-0 
RimWellLogTrackTools.cpp
New static utility tools for well log track operations     

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackTools.cpp

  • New utility class with static methods for well log track operations
  • Provides curve sampling point data extraction for Eclipse and GeoMech
    extractors
  • Implements region name finding, formation vector retrieval, and
    overburden/underburden handling
  • Includes simulation well extractor creation and range adjustment
    utilities
  • Handles ASCII export formatting for plot data
+484/-0 
RimWellLogTrackStackedCurves.cpp
New stacked curves management helper class                             

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackStackedCurves.cpp

  • New file managing stacked curve data updates and visibility
  • Implements curve stacking logic with proper depth range handling
  • Handles phase-based coloring and z-order positioning for stacked
    curves
  • Provides visible stacked curves filtering and data resampling
+188/-0 
RimWellLogTrackPropertyAxis.h
New property axis helper class header                                       

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackPropertyAxis.h

  • New helper class header for managing property value axis on well log
    tracks
  • Declares public methods for calculating zoom range, updating axis, and
    managing tick intervals
  • Provides interface for logarithmic scale handling and axis scale
    engine configuration
+40/-0   
RimWellLogTrackPropertyAxis.cpp
Property axis management implementation                                   

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackPropertyAxis.cpp

  • Implements property axis management with 371 lines of functionality
  • Handles zoom range calculation for property values with topology curve
    special handling
  • Manages axis tick intervals, grid lines, and scale engine
    configuration (linear/logarithmic)
  • Includes logarithmic scale minimum value computation for proper axis
    display
+371/-0 
RimWellLogTrackStackedCurves.h
New stacked curves helper class header                                     

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackStackedCurves.h

  • New helper class header for managing stacked curves visualization
  • Declares methods for updating stacked curve data and retrieving
    visible curves
  • Provides interface to access curve stacking information by index
+41/-0   
RimWellLogTrackWellPathComponents.h
New well path components helper class header                         

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackWellPathComponents.h

  • New helper class header for managing well path component visualization
  • Declares methods for updating well path attributes and completions on
    plot
  • Provides interface for managing plot item attachment/detachment and
    depth range queries
+49/-0   
RimWellLogTrackWellPathComponents.cpp
Well path components management implementation                     

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackWellPathComponents.cpp

  • Implements well path component visualization with 222 lines of
    functionality
  • Manages well path attributes, completions, and their plot
    representation
  • Handles component sorting by type and legend contribution logic
  • Provides depth range calculation across all well path component plot
    items
+222/-0 
RimWellLogTrackRegionAnnotations.h
New region annotations helper class header                             

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackRegionAnnotations.h

  • New helper class header for managing region annotations on well log
    tracks
  • Declares methods for updating and removing region annotations
  • Provides interface for water and rock region calculation from
    geomechanical extractors
+54/-0   
RimWellLogTrackTools.h
New well log track utility functions header                           

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackTools.h

  • New utility class header with static helper functions for well log
    track operations
  • Provides curve sampling, formation name retrieval, and region name
    plotting utilities
  • Includes axis range adjustment functions and ASCII export
    functionality
  • Supports both eclipse and geomechanical well log extraction
+82/-0   
Configuration changes
1 files
CMakeLists_files.cmake
Updated CMake build configuration for new files                   

ApplicationLibCode/ProjectDataModel/WellLog/CMakeLists_files.cmake

  • Added five new header files to SOURCE_GROUP_HEADER_FILES
  • Added five new source files to SOURCE_GROUP_SOURCE_FILES
  • Maintains alphabetical ordering within the CMake configuration
+10/-0   

magnesj and others added 4 commits January 23, 2026 20:12
Moves region annotation logic into a dedicated
RimWellLogTrackRegionAnnotations class for better
organization and maintainability. It also introduces
RimWellLogTrackTools for static utility functions.

This change improves code readability and allows for
easier extension of region annotation features.
@qodo-code-review
Copy link

qodo-code-review bot commented Jan 26, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Denial of service

Description: Potential denial-of-service via unbounded/invalid allocation: maxCategoryValue is
initialized to std::numeric_limits::min() and then used to size std::vector
namesVector(maxCategoryValue + 1), which can become an enormous size (or effectively wrap)
if colorLegendItems() is empty or contains a very large categoryValue(), leading to
bad_alloc/crash.
RimWellLogTrackRegionAnnotations.cpp [389-401]

Referred Code
// Find the largest category number.
int maxCategoryValue = std::numeric_limits<int>::min();
for ( RimColorLegendItem* legendItem : colorShadingLegend->colorLegendItems() )
{
    maxCategoryValue = std::max( maxCategoryValue, legendItem->categoryValue() );
}

// Insert each name at index matching the category number.
std::vector<QString> namesVector( maxCategoryValue + 1 );
for ( RimColorLegendItem* legendItem : colorShadingLegend->colorLegendItems() )
{
    namesVector[legendItem->categoryValue()] = legendItem->categoryName();
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null checks: New code dereferences potentially null objects (e.g., m_track->resultDefinition(),
dynamic_cast<RimEclipseCase*>(formationCase), and eclipseCase->eclipseCaseData())
without validating pointers, risking crashes instead of graceful handling.

Referred Code
RimWellPath* formationWellPathForSourceCase = m_track->formationWellPath();
RimCase*     formationCase                  = m_track->formationNamesCase();
QString      formationSimWellName           = m_track->formationSimWellName();

RigEclipseWellLogExtractor* eclWellLogExtractor =
    RiaExtractionTools::findOrCreateWellLogExtractor( formationWellPathForSourceCase, dynamic_cast<RimEclipseCase*>( formationCase ) );

if ( !eclWellLogExtractor )
{
    RiaLogging::error( "No well log extractor found for case." );
    return;
}

RimEclipseCase* eclipseCase = dynamic_cast<RimEclipseCase*>( formationCase );

RimEclipseResultDefinition* resultDefinition = m_track->resultDefinition();
resultDefinition->loadResult();

size_t                      timeStep = 0;
cvf::ref<RigResultAccessor> resultAccessor =
    RigResultAccessorFactory::createFromResultDefinition( eclipseCase->eclipseCaseData(), 0, timeStep, resultDefinition );


 ... (clipped 12 lines)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 26, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Re-evaluate the refactoring strategy
Suggestion Impact:The commit removed many of the public accessor methods that existed primarily to serve helper classes (property axis/region annotations/well path components), aligning with the suggestion’s concern about exposing internal state via lots of accessors. However, instead of decoupling helpers from the track, the helpers still hold a RimWellLogTrack* and were made friends so they can access private members directly, which keeps (and arguably increases) coupling rather than replacing it with parameter/context passing.

code diff:

+    friend class RimWellLogTrackPropertyAxis;
+    friend class RimWellLogTrackRegionAnnotations;
+    friend class RimWellLogTrackStackedCurves;
+    friend class RimWellLogTrackWellPathComponents;
+
 public:
     enum TrajectoryType
     {
@@ -198,41 +203,11 @@
 
     RimWellPath* wellPathAttributeSource() const;
 
-    // Accessors for region annotation helper class
-    FormationSource                       formationSource() const;
-    RimWellPath*                          formationWellPathForSourceWellPath() const;
-    RigWellPathFormations::FormationLevel formationLevel() const;
-    bool                                  showFormationFluids() const;
-    int                                   colorShadingTransparency() const;
-    bool                                  showRegionLabels() const;
-    RimColorLegend*                       colorShadingLegend() const;
-    RimEclipseResultDefinition*           resultDefinition() const;
-    caf::FontTools::RelativeSize          regionLabelFontSize() const;
-
     void setLogarithmicScale( bool enable );
     bool isLogarithmicScale() const;
 
-    // Property axis accessors for helper class
-    double minorTickIntervalPropertyAxis() const;
-    double majorTickIntervalPropertyAxis() const;
-    void   setAvailablePropertyValueRange( double minValue, double maxValue );
-    bool   isAutoScalePropertyValuesEnabled() const;
-    bool   isPropertyValueAxisInverted() const;
-    bool   isPropertyAxisMinAndMaxTicksOnly() const;
-    bool   isExplicitTickIntervalsPropertyValueAxis() const;
-    int    propertyValueAxisGridVisibility() const;
-    bool   showWellPathComponentsBothSides() const;
-    bool   showWellPathComponentLabels() const;
-    bool   isEmptyVisiblePropertyRange() const;
-
     RiuPlotAxis depthAxis() const;
     RiuPlotAxis valueAxis() const;
-
-    // Well path component accessors for helper class
-    bool                            showWellPathCompletions() const;
-    bool                            wellPathCompletionsInLegend() const;
-    RimWellPathAttributeCollection* wellPathAttributeCollection() const;
-    void                            setWellPathAttributeCollection( RimWellPathAttributeCollection* attributeCollection );
 
     void updatePropertyValueZoom();
 
@@ -342,6 +317,8 @@
     void doUpdateLayout() override;
 
     void connectCurveSignals( RimWellLogCurve* curve );
+
+    bool isEmptyVisiblePropertyRange() const;
 
 private:
     caf::PdmField<QString>                    m_description;
@@ -399,11 +376,11 @@
 
     bool m_formationsForCaseWithSimWellOnly;
 
-    QPointer<RiuWellLogTrack>                             m_plotWidget;
-    std::unique_ptr<RimWellLogTrackRegionAnnotations>    m_regionAnnotations;
-    std::unique_ptr<RimWellLogTrackPropertyAxis>         m_propertyAxis;
-    std::unique_ptr<RimWellLogTrackStackedCurves>        m_stackedCurves;
-    std::unique_ptr<RimWellLogTrackWellPathComponents>   m_wellPathComponents;
+    QPointer<RiuWellLogTrack>                          m_plotWidget;
+    std::unique_ptr<RimWellLogTrackRegionAnnotations>  m_regionAnnotations;
+    std::unique_ptr<RimWellLogTrackPropertyAxis>       m_propertyAxis;
+    std::unique_ptr<RimWellLogTrackStackedCurves>      m_stackedCurves;
+    std::unique_ptr<RimWellLogTrackWellPathComponents> m_wellPathComponents;
 
     QString m_propertyValueAxisTitle;
     double  m_availablePropertyValueRangeMin;

# File: ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackPropertyAxis.cpp
@@ -102,9 +102,9 @@
         minValue = 0;
         maxValue = 0;
     }
-    else if ( m_track->minorTickIntervalPropertyAxis() != 0.0 )
-    {
-        std::tie( minValue, maxValue ) = RimWellLogTrackTools::adjustXRange( minValue, maxValue, m_track->minorTickIntervalPropertyAxis() );
+    else if ( m_track->m_minorTickIntervalPropertyAxis != 0.0 )
+    {
+        std::tie( minValue, maxValue ) = RimWellLogTrackTools::adjustXRange( minValue, maxValue, m_track->m_minorTickIntervalPropertyAxis );
     }
     else
     {
@@ -115,7 +115,8 @@
         maxValue = adjustedMax;
     }
 
-    m_track->setAvailablePropertyValueRange( minValue, maxValue );
+    m_track->m_availablePropertyValueRangeMin = minValue;
+    m_track->m_availablePropertyValueRangeMax = maxValue;
 }
 
 //--------------------------------------------------------------------------------------------------
@@ -132,7 +133,7 @@
     double availableMax = 0.0;
     m_track->availablePropertyValueRange( &availableMin, &availableMax );
 
-    if ( m_track->isAutoScalePropertyValuesEnabled() )
+    if ( m_track->m_isAutoScalePropertyValuesEnabled )
     {
         double visibleMin = availableMin;
         double visibleMax = availableMax;
@@ -157,12 +158,12 @@
     // Set an extended range here to allow for some label space.
     double componentRangeMax = 2.0 / ( static_cast<double>( m_track->colSpan() ) );
     double componentRangeMin = -0.25;
-    if ( m_track->showWellPathComponentsBothSides() )
+    if ( m_track->m_showWellPathComponentsBothSides )
     {
         componentRangeMin = -1.5;
         componentRangeMax *= 2.0;
     }
-    if ( m_track->showWellPathComponentLabels() )
+    if ( m_track->m_showWellPathComponentLabels )
     {
         componentRangeMax *= 1.5;
     }
@@ -191,7 +192,9 @@
     RiuPlotAxis valueAxis = m_track->valueAxis();
     RiuPlotAxis depthAxis = m_track->depthAxis();
 
-    bool emptyRange = m_track->isEmptyVisiblePropertyRange();
+    bool emptyRange =
+        std::abs( m_track->m_visiblePropertyValueRangeMax - m_track->m_visiblePropertyValueRangeMin ) <
+        1.0e-6 * std::max( 1.0, std::max( m_track->m_visiblePropertyValueRangeMax.value(), m_track->m_visiblePropertyValueRangeMin.value() ) );
     if ( emptyRange )
     {
         plotWidget->enableGridLines( valueAxis, false, false );
@@ -208,9 +211,9 @@
 
         auto rangeBoundaryA = visibleMin;
         auto rangeBoundaryB = visibleMax;
-        if ( m_track->isPropertyValueAxisInverted() ) std::swap( rangeBoundaryA, rangeBoundaryB );
-
-        if ( m_track->isPropertyAxisMinAndMaxTicksOnly() )
+        if ( m_track->m_invertPropertyValueAxis ) std::swap( rangeBoundaryA, rangeBoundaryB );
+
+        if ( m_track->m_propertyAxisMinAndMaxTicksOnly )
         {
             auto roundToDigits = []( double value, int numberOfDigits, bool useFloor )
             {
@@ -255,11 +258,11 @@
                 plotWidget->qwtPlot()->setAxisScaleDiv( QwtAxis::YLeft, div );
             }
         }
-        else if ( m_track->isExplicitTickIntervalsPropertyValueAxis() )
+        else if ( m_track->m_explicitTickIntervalsPropertyValueAxis )
         {
             plotWidget->setMajorAndMinorTickIntervals( valueAxis,
-                                                       m_track->majorTickIntervalPropertyAxis(),
-                                                       m_track->minorTickIntervalPropertyAxis(),
+                                                       m_track->m_majorTickIntervalPropertyAxis,
+                                                       m_track->m_minorTickIntervalPropertyAxis,
                                                        rangeBoundaryA,
                                                        rangeBoundaryB );
         }
@@ -272,8 +275,8 @@
         }
 
         plotWidget->enableGridLines( valueAxis,
-                                     m_track->propertyValueAxisGridVisibility() & RimWellLogPlot::AXIS_GRID_MAJOR,
-                                     m_track->propertyValueAxisGridVisibility() & RimWellLogPlot::AXIS_GRID_MINOR );
+                                     m_track->m_propertyValueAxisGridVisibility.value() & RimWellLogPlot::AXIS_GRID_MAJOR,
+                                     m_track->m_propertyValueAxisGridVisibility.value() & RimWellLogPlot::AXIS_GRID_MINOR );
     }
 
     RimDepthTrackPlot* wellLogPlot = m_track->firstAncestorOrThisOfType<RimDepthTrackPlot>();
@@ -346,7 +349,7 @@
 //--------------------------------------------------------------------------------------------------
 void RimWellLogTrackPropertyAxis::computeAndSetPropertyValueRangeMinForLogarithmicScale()
 {
-    if ( m_track->isAutoScalePropertyValuesEnabled() && m_track->isLogarithmicScale() )
+    if ( m_track->m_isAutoScalePropertyValuesEnabled && m_track->isLogarithmicScale() )
     {

The refactoring creates tightly coupled helper classes that depend on the main
class. A better approach would be to make these helpers more independent by
passing data through method parameters instead of giving them a pointer to the
main class, which would improve encapsulation and reduce coupling.

Examples:

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrack.h [199-236]
    RimWellPath* wellPathAttributeSource() const;

    // Accessors for region annotation helper class
    FormationSource                       formationSource() const;
    RimWellPath*                          formationWellPathForSourceWellPath() const;
    RigWellPathFormations::FormationLevel formationLevel() const;
    bool                                  showFormationFluids() const;
    int                                   colorShadingTransparency() const;
    bool                                  showRegionLabels() const;
    RimColorLegend*                       colorShadingLegend() const;

 ... (clipped 28 lines)
ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackPropertyAxis.cpp [49-51]
RimWellLogTrackPropertyAxis::RimWellLogTrackPropertyAxis( RimWellLogTrack* track )
    : m_track( track )
{

Solution Walkthrough:

Before:

class RimWellLogTrack {
public:
    void updatePropertyValueZoom() {
        // ... complex logic ...
    }
    void updateRegionAnnotationsOnPlot() {
        // ... complex logic ...
    }
    // ... many other large methods ...

private:
    // ... many private members ...
    std::unique_ptr<RimWellLogTrackPropertyAxis> m_propertyAxis;
    std::unique_ptr<RimWellLogTrackRegionAnnotations> m_regionAnnotations;
    // ... other helpers ...
};

// In constructor:
m_propertyAxis = std::make_unique<RimWellLogTrackPropertyAxis>(this);

// In implementation:
void RimWellLogTrack::updatePropertyValueZoom() {
    m_propertyAxis->updatePropertyValueZoom();
}

After:

class RimWellLogTrack {
public:
    void updatePropertyValueZoom() {
        // Gather necessary data from private members
        auto data = createPropertyAxisContext();
        m_propertyAxis->updatePropertyValueZoom(data);
    }
    // ... other methods delegating with context objects ...

private:
    PropertyAxisContext createPropertyAxisContext() {
        // ... create context object with needed data ...
    }
    // ... fewer or no public accessors for helpers ...
    std::unique_ptr<RimWellLogTrackPropertyAxis> m_propertyAxis;
};

class RimWellLogTrackPropertyAxis {
public:
    // No longer holds a pointer to RimWellLogTrack
    void updatePropertyValueZoom(const PropertyAxisContext& context) {
        // Logic uses data from context, not by calling back to track
    }
};
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a fundamental design issue where the refactoring introduces tight coupling and breaks encapsulation by adding numerous public accessors, which undermines the goal of improved maintainability.

High
Possible issue
Avoid potential crash with null check

Replace firstAncestorOrThisOfTypeAsserted() with the safer
firstAncestorOrThisOfType() and add a null check for the returned pointer to
prevent potential crashes.

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackRegionAnnotations.cpp [160-170]

 void RimWellLogTrackRegionAnnotations::updateFormationNamesOnPlot()
 {
-    RimDepthTrackPlot* plot = m_track->firstAncestorOrThisOfTypeAsserted<RimDepthTrackPlot>();
+    RimDepthTrackPlot* plot = m_track->firstAncestorOrThisOfType<RimDepthTrackPlot>();
+    if ( !plot ) return;
 
     RiaDefines::DepthUnitType fromDepthUnit = plot->caseDepthUnit();
     RiaDefines::DepthUnitType toDepthUnit   = plot->depthUnit();
 
     auto orientation = plot->depthOrientation();
 
     RiuQwtPlotWidget* plotWidget = m_track->viewer();
     if ( !plotWidget ) return;
 ...

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential crash due to firstAncestorOrThisOfTypeAsserted and proposes a safer alternative with a null check, which is a significant improvement for robustness.

Medium
Add null check for pointer argument

Add a null check for the depthTrackPlot parameter at the beginning of the
asciiDataForPlotExport function to prevent potential null pointer dereferences.

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackTools.cpp [339-358]

 QString RimWellLogTrackTools::asciiDataForPlotExport( const QString&                       trackDescription,
                                                        RimDepthTrackPlot*                   depthTrackPlot,
                                                        const std::vector<RimWellLogCurve*>& curves )
 {
+    if ( !depthTrackPlot ) return QString();
+
     QString out = "\n" + trackDescription + "\n";
 
     std::vector<QString>             curveNames;
     std::vector<double>              curveDepths;
     std::vector<std::vector<double>> curvesPlotXValues;
 
     auto depthType             = depthTrackPlot->depthType();
     auto depthUnit             = depthTrackPlot->depthUnit();
     bool isWellAllocInflowPlot = false;
     {
         auto wapl = depthTrackPlot->firstAncestorOfType<RimWellAllocationPlot>();
         if ( wapl )
         {
             isWellAllocInflowPlot = ( wapl->flowType() == RimWellAllocationPlot::INFLOW );
         }
     }
 ...

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a missing null check for the depthTrackPlot pointer, which is dereferenced multiple times. Adding this check prevents a potential crash, improving the code's robustness.

Medium
Add branchIndex bounds check

Replace the CVF_ASSERT with an explicit bounds check for branchIndex before
accessing wellPaths to prevent out-of-bounds access in release builds.

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackTools.cpp [273-278]

-CVF_ASSERT( branchIndex >= 0 && branchIndex < static_cast<int>( wellPaths.size() ) );
-return ( wellLogCollection->findOrCreateSimWellExtractor( simWellName,
-                                                          QString( "Find or create sim well extractor" ),
-                                                          wellPaths[branchIndex],
-                                                          eclipseCase->eclipseCaseData() ) );
+if ( branchIndex < 0 || branchIndex >= static_cast<int>( wellPaths.size() ) )
+    return nullptr;
+return wellLogCollection->findOrCreateSimWellExtractor( simWellName,
+                                                        QString( "Find or create sim well extractor" ),
+                                                        wellPaths[branchIndex],
+                                                        eclipseCase->eclipseCaseData() );
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly points out that CVF_ASSERT is disabled in release builds, and an out-of-bounds access could occur. Adding an explicit check for branchIndex makes the code safer and more robust.

Medium
Increment phase color index

Increment stackIndex within the loop to ensure each stacked curve is assigned a
unique color index for correct phase color visualization.

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackStackedCurves.cpp [117-158]

 size_t              stackIndex = 0u;
 std::vector<double> allStackedValues( allDepthValues.size(), 0.0 );
 for ( auto curve : stackedCurvesInGroup )
 {
     ...
     if ( curve->isStackedWithPhaseColors() )
     {
         curve->assignStackColor( stackIndex, curvePhaseCount[curve->phaseType()] );
     }
+    ++stackIndex;
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: This suggestion fixes a bug where stackIndex was not incremented, causing all stacked curves to be assigned the same color index, which is incorrect visual behavior.

Medium
Prevent crash from missing map key

Replace sortIndices.at() with a safer lookup using sortIndices.find() to prevent
potential crashes from missing keys in the sort map.

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackWellPathComponents.cpp [113-116]

 std::stable_sort( allWellPathComponents.begin(),
                   allWellPathComponents.end(),
-                  [&sortIndices]( const RimWellPathComponentInterface* lhs, const RimWellPathComponentInterface* rhs )
-                  { return sortIndices.at( lhs->componentType() ) < sortIndices.at( rhs->componentType() ); } );
+                  [&sortIndices]( const RimWellPathComponentInterface* lhs, const RimWellPathComponentInterface* rhs ) {
+                      auto lhsIt = sortIndices.find( lhs->componentType() );
+                      auto rhsIt = sortIndices.find( rhs->componentType() );
+                      int  lhsIndex =
+                          ( lhsIt != sortIndices.end() ) ? lhsIt->second : std::numeric_limits<int>::max();
+                      int rhsIndex =
+                          ( rhsIt != sortIndices.end() ) ? rhsIt->second : std::numeric_limits<int>::max();
+                      return lhsIndex < rhsIndex;
+                  } );
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential crash risk when using map::at() and provides a more robust solution using map::find() to handle unknown component types gracefully.

Medium
General
Avoid multiple large vector copies

Improve performance by using a pointer to track the vector with the largest
depth range, avoiding multiple large vector copies within the loop.

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackStackedCurves.cpp [92-113]

+const std::vector<double>* masterDepths = nullptr;
 for ( auto curve : stackedCurvesInGroup )
 {
-    auto depths = curve->curveData()->depths( depthType );
+    const auto& depths = curve->curveData()->depths( depthType );
     if ( depths.empty() ) continue;
 
-    if ( allDepthValues.empty() )
+    if ( masterDepths == nullptr )
     {
+        masterDepths  = &depths;
         auto minmaxit = std::minmax_element( depths.begin(), depths.end() );
         maxDepthRange = std::make_pair( *minmaxit.first, *minmaxit.second );
-        allDepthValues.insert( allDepthValues.end(), depths.begin(), depths.end() );
     }
     else
     {
         auto                      minmaxit   = std::minmax_element( depths.begin(), depths.end() );
         std::pair<double, double> depthRange = std::make_pair( *minmaxit.first, *minmaxit.second );
         if ( std::fabs( depthRange.second - depthRange.first ) > std::fabs( maxDepthRange.second - maxDepthRange.first ) )
         {
-            maxDepthRange  = depthRange;
-            allDepthValues = depths;
+            maxDepthRange = depthRange;
+            masterDepths  = &depths;
         }
     }
 }
 
+if ( !masterDepths ) continue;
+allDepthValues = *masterDepths;
+
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion provides a valid performance optimization by avoiding potentially expensive vector copies inside a loop, which improves efficiency for large datasets.

Low
Share single scale engine instance

Create a single scale engine instance and share it between the top and bottom
axes to improve efficiency and ensure consistency.

ApplicationLibCode/ProjectDataModel/WellLog/RimWellLogTrackPropertyAxis.cpp [307-320]

 if ( m_track->isLogarithmicScale() )
 {
-    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XTop, new QwtLogScaleEngine );
+    auto scaleEngine = new QwtLogScaleEngine;
+    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XTop, scaleEngine );
 
     // NB! Must assign scale engine to bottom in order to make QwtPlotGrid work
-    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XBottom, new QwtLogScaleEngine );
+    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XBottom, scaleEngine );
 }
 else
 {
-    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XTop, new RiuQwtLinearScaleEngine );
+    auto scaleEngine = new RiuQwtLinearScaleEngine;
+    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XTop, scaleEngine );
 
     // NB! Must assign scale engine to bottom in order to make QwtPlotGrid work
-    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XBottom, new RiuQwtLinearScaleEngine );
+    plotWidget->qwtPlot()->setAxisScaleEngine( QwtAxis::XBottom, scaleEngine );
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that sharing a single scale engine instance is more efficient and robust than creating two separate ones for related axes.

Low
  • Update

Refactored RimWellLogTrack by removing numerous public accessor methods for private members related to formation source, property axis, region annotation, and well path components. Helper classes now access these members directly via friend declarations. Updated all usage in helper classes to reference member variables directly, simplifying the class interface and reducing the public API surface for improved encapsulation. Moved isEmptyVisiblePropertyRange() to private scope and implemented it inline. Ensured consistent behavior across all affected logic.
@magnesj magnesj force-pushed the refactor-well-log-track branch from 950352f to a61c704 Compare January 26, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant