-
Notifications
You must be signed in to change notification settings - Fork 74
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
Closed regions search refactoring #992
Closed regions search refactoring #992
Conversation
Add NetworkNode class that contains vertex Id and vertex Position at the same time.
Moved enum VisitDirections to separate file because it will be used in Local Edge and NetworkEdge later.
NetworkEdge class is similar to LocalEdge class from Network.cs, but it uses NetworkNode instead if integer node index. NetworkEdge also doesn't have IsBetweenVertices method.
These methods are required for the future code simplification.
- Moved cycles search logic to the new NetworkCycleCoverage class; - No changes in logic; - Removed private methods that are no longer used from the Network class; - Added a constructor to the NetworkCycleCoverage class that receives adjacencyList from Network.
CreateAdjacencyMatrixWithPositionInfo constructs adjacency matrix based on NetworkNode rather than on integer node index and T. Such adjacency matrix simplifies the future code significantly because: - Now we can just remove edges and nodes that we don't want to include into search. Such changes also don't affect the original matrix, so it may be used later for other purpose. - Now edge (i,j) that is attached to node i is THE SAME edge that is attached to node j.
- Switch from _adjacencyList to _adjacencyMatrix. - The code passes all the Network tests after these changes. - It is no longer required to scan all the edges in order to find one. NetworkEdge (i,j) that is in _adjacencyMatrix[i] is the same edge as the one in _adjacencyMatrix[j]. So it is only required to look through the edges in _adjacencyMatrix[i] or _adjacencyMatrix[j] to find it. - Search of edges and nodes positions does not require allNodeLocations and allEdges collections anymore. - TraverseLargestPlaneAngle is not static anymore and isn't passed as parameter through all the methods. - Returned and updated summary for TraverseLargestPlaneAngle and Traverse methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One little comment.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @srudenkoamc)
Elements/src/Search/NetworkCycleCoverage.cs
line 192 at r1 (raw file):
var edge = GetEdge(path[j], path[j + 1]); // if (edge == null)
Let's remove this commented code. Even though it was there previously, I don't think it should be.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained
Elements/src/Search/NetworkCycleCoverage.cs
line 192 at r1 (raw file):
Done
BACKGROUND:
DESCRIPTION:
TESTING:
FUTURE WORK:
This change is