Conversation
eivindjahren
force-pushed
the
refactor_layer
branch
4 times, most recently
from
6ce0786 to
859978d
Compare
There was a problem hiding this comment.
Pull request overview
This PR modernizes the layer implementation by refactoring from C to C++, adding comprehensive test coverage, and improving code maintainability. The changes convert legacy C-style memory management to modern C++ patterns while preserving the public API for compatibility with existing C code.
Changes:
- Refactored
layer.cppfrom C to C++ using modern patterns (std::vector, new/delete, member functions) - Added comprehensive test suite
test_layer.cppwith 663 lines covering layer functionality - Updated CMakeLists.txt to include new test file and private headers
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| lib/resdata/layer.cpp | Modernized from C to C++ with std::vector replacing raw arrays, new/delete instead of malloc/free, member functions for encapsulation, and BlockTracer class for cleaner recursive tracing |
| lib/tests/test_layer.cpp | Comprehensive test suite covering getters/setters, barriers, edge tracing, cell operations, and grid integration with multiple edge cases |
| lib/CMakeLists.txt | Added test_layer.cpp to test suite and included private headers directory for internal API access |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eivindjahren
force-pushed
the
refactor_layer
branch
from
859978d to
b0f4cd0
Compare
eivindjahren
force-pushed
the
refactor_layer
branch
2 times, most recently
from
5fc8dfa to
329525c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eivindjahren
force-pushed
the
refactor_layer
branch
5 times, most recently
from
a7042fa to
99b5a62
Compare
eivindjahren
force-pushed
the
refactor_layer
branch
2 times, most recently
from
4c202e5 to
8dd83be
Compare
eivindjahren
force-pushed
the
refactor_layer
branch
from
8dd83be to
ecdfbeb
Compare
ajaust
left a comment
ajaust
left a comment
There was a problem hiding this comment.
Looks great. I also like the really small commit. 🙂
I have only added minor comments and ideas on how to sprinkle more STL on the file. You are free to decide what makes sense here.
lib/resdata/layer.cpp
Outdated
|
|
||
| static int layer_get_global_cell_index__(const layer_type *layer, int i, | ||
| int j) { | ||
| static int global_cell_index(const layer_type *layer, int i, int j) { |
There was a problem hiding this comment.
In the previous commit, the renaming to global_interior_index dropped cell from the original name. In this commit, cell is still part of the name. Is there are difference between the two, i.e., is the global_interior_index not referring to a cell or is it clear from context?
eivindjahren
force-pushed
the
refactor_layer
branch
from
ecdfbeb to
6026bbe
Compare
No description provided.