Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use min value instead of average value to order bus cells #462

Merged
merged 5 commits into from
Jan 25, 2023

Conversation

So-Fras
Copy link
Member

@So-Fras So-Fras commented Jan 9, 2023

Signed-off-by: Sophie Frasnedo sophie.frasnedo@rte-france.com

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature

What is the current behavior? (You can also link to an open issue here)
The position of the cell on the bus bar is determined by the average position numbers of its components.

What is the new behavior (if this is a feature change)?
The position of the cell on the bus bar is determined by the minimum position number among its components.

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
@So-Fras So-Fras requested a review from flo-dup January 9, 2023 09:11
Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at previous lines below, it seems that they are unneeded, maybe we can take the opportunity to remove them

graph.getNodes().stream().filter(node -> node.getDirection() != Direction.UNDEFINED && graph.getCell(node).isPresent()).forEach(node -> {
BusCell cell = (BusCell) graph.getCell(node).get();
cell.setDirection(node.getDirection());
node.getOrder().ifPresent(cell::setOrder);
});

@So-Fras So-Fras self-assigned this Jan 11, 2023
@So-Fras So-Fras added the SLD label Jan 12, 2023
@flo-dup
Copy link
Contributor

flo-dup commented Jan 19, 2023

In fact the mentioned lines should be removed but I forgot one thing: you should set the direction of the bus cell inside the graph.getBusCellStream().forEach() part. Currently, in the mentioned lines, the last node in the stream decide the direction if there are several different directions, and this is bad. You should check that the directions are coherent, and log a warning (throw an exception is maybe too harsh) if they're not.

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
@So-Fras So-Fras requested a review from flo-dup January 19, 2023 15:27
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

82.4% 82.4% Coverage
0.0% 0.0% Duplication

@flo-dup flo-dup merged commit b9649cb into main Jan 25, 2023
@flo-dup flo-dup deleted the position_order_logic_modification branch January 25, 2023 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants