Refactor modeler code #2616
Refactor modeler code #2616
Conversation
pet-mit
commented
Feb 3, 2025
•
pet-mit
commented
- move "expressions" code to src/expressions
- move optimisation sources to src/optimisatio,
- move yml importers to src/io
Signed-off-by: Peter Mitri <[email protected]>
Signed-off-by: Peter Mitri <[email protected]>
Signed-off-by: Peter Mitri <[email protected]>
|
this line must be updated in order to avoid sonar issues: |
| @@ -3,7 +3,7 @@ | |||
| #include <stack> | |||
| #include <vector> | |||
|
|
|||
| namespace Antares::Solver::Nodes | |||
There was a problem hiding this comment.
Why is this file named pre-order.h whereas it contains declaration of class AST ?
AST seems to be the main topic here, at least the feature we should highlight.
Class ASTPreOrderIterator is an implementation detail.
Rename into ast.h ?
| ) | ||
|
|
||
|
|
||
| add_library(expressions-iterators |
There was a problem hiding this comment.
This remark follows this one.
About expressions-iterators target : as it mainly defines class AST, and AST is central to expressions, these files should be part of expressions target.
| @@ -22,11 +22,11 @@ | |||
| #pragma once | |||
| #include <stdexcept> | |||
|
|
|||
| namespace Antares::Solver::Visitors | |||
| namespace Antares::Expressions::Visitors | |||
| { | |||
| class InvalidNode: public std::invalid_argument | |||
| { | |||
| public: | |||
| explicit InvalidNode(const std::string& node_name = ""); | |||
There was a problem hiding this comment.
Dangerous to have a default argument here (node_name = "").
This means we allow :
throw InvalidNode();without specifying any node name.
In this case, if exception is caught, associated error message will be cryptic, as we don't know which node is concerned.
| { | ||
|
|
||
| InvalidNode::InvalidNode(const std::string& node_name): | ||
| std::invalid_argument("Antares::Expressions::Nodes Visitor: invalid node type " + node_name) |
There was a problem hiding this comment.
"Antares::Expressions::Nodes Visitor: ..."Does user cares about hard coded namespace things when she reads the error message of an invalid node ?
"Invalid node type " + node_nameis enough
There was a problem hiding this comment.
I made it more "human":
"Node visitor encountered an invalid node type: " + node_name
|
Attempted small changes on the current PR : see #2619 |
Signed-off-by: Peter Mitri <[email protected]>
|
pet-mit
dismissed
guilpier-code’s stale review
will be merging other PR into develop
