Get switches and internal connections from given node (#391) #422
Conversation
sebalaig
force-pushed
the
issue391-get-switched-internalConnection
branch
from
460504f to
8d633cc
Compare
| const_range<unsigned long> getEdges() const; | ||
|
|
||
| unsigned long getMaxVertex() const; | ||
|
|
||
| unsigned long getVertex1(unsigned long e) const; | ||
| const unsigned long& getVertex1(unsigned long e) const; |
There was a problem hiding this comment.
To be discussed: I think it's not wrong, I had to update that in order to return a range in NodeBreakerViewImpl::getNodesInternalConnectedTo (unless it was gardening...). But maybe NodeBreakerViewImpl::getNodesInternalConnectedTo should return a vector instead ?
There was a problem hiding this comment.
I have to think a little bit more about the issue you encountered
sebalaig
force-pushed
the
issue391-get-switched-internalConnection
branch
from
a04401d to
7a18e03
Compare
sebalaig
changed the title
| @@ -54,19 +54,29 @@ class UndirectedGraph { | |||
|
|
|||
| const stdcxx::Reference<E>& getEdgeObject(unsigned long e) const; | |||
|
|
|||
| const_range<E> getEdgeObjectsConnectedToVertex(unsigned long vertex) const; | |||
|
|
|||
| const std::vector<unsigned long>& getEdgeConnectedToVertex(unsigned long vertex) const; | |||
There was a problem hiding this comment.
I'm not sure we have to keep this method. It seems you backport the IntStream getEdgeConnectedToVertexStream(int v); method, but from my point of view, an S is missing in the name of the method.
| const_range<E> getEdgeObjects() const; | ||
|
|
||
| range<E> getEdgeObjects(); | ||
|
|
||
| const_range<E> getEdgeObjects(unsigned long v1, unsigned long v2) const; | ||
|
|
||
| const unsigned long& getEdgeVertex1(unsigned long edge) const; | ||
|
|
||
| const unsigned long& getEdgeVertex2(unsigned long edge) const; |
There was a problem hiding this comment.
To be discussed: not sure this is a good idea to return a reference here.
| const_range<unsigned long> getEdges() const; | ||
|
|
||
| unsigned long getMaxVertex() const; | ||
|
|
||
| unsigned long getVertex1(unsigned long e) const; | ||
| const unsigned long& getVertex1(unsigned long e) const; |
There was a problem hiding this comment.
I have to think a little bit more about the issue you encountered
| @@ -12,6 +12,7 @@ | |||
| #include <set> | |||
| #include <string> | |||
|
|
|||
| #include <powsybl/PowsyblException.hpp> | |||
There was a problem hiding this comment.
I'm not sure you should add this include in a header.
|
|
||
| const auto& mapper = [this, node](const unsigned long& edge) -> const unsigned long& { | ||
| unsigned long vertex1 = m_voltageLevel.getGraph().getEdgeVertex1(edge); | ||
| return vertex1 != node ? m_voltageLevel.getGraph().getEdgeVertex1(edge) : m_voltageLevel.getGraph().getEdgeVertex2(edge); |
There was a problem hiding this comment.
We should avoid this useless call to m_voltageLevel.getGraph().getEdgeVertex1(edge)
Signed-off-by: Sébastien LAIGRE <[email protected]>
Signed-off-by: Sébastien LAIGRE <[email protected]>
Signed-off-by: Sébastien LAIGRE <[email protected]>
sebalaig
force-pushed
the
issue391-get-switched-internalConnection
branch
from
7a18e03 to
d7687f2
Compare
|
SonarCloud Quality Gate failed. |
|
Kudos, SonarCloud Quality Gate passed! |
Signed-off-by: Sébastien LAIGRE [email protected]
Please check if the PR fulfills these requirements (please use
'[x]'to check the checkboxes, or submit the PR and then click the checkboxes)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'and skip the restCloses #391
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change or deprecate an API? If yes, check the following:
Other information:
(if any of the questions/checkboxes don't apply, please delete them entirely)