8000 Adds link vertex get/set functions to the API by LRossman · Pull Request #549 · OpenWaterAnalytics/EPANET · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Adds link vertex get/set functions to the API #549

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 31, 2019
Merged

Adds link vertex get/set functions to the API #549

merged 3 commits into from
Oct 31, 2019

Conversation

LRossman
Copy link
Collaborator

See issue #548.

@eladsal
Copy link
Member
eladsal commented Oct 30, 2019

Thank you @LRossman. Is there a reason you added an API to get a single vertex point but the set API is for the entire array of vertices? Shouldn't we have the two options as we have, as an example, for the curves data?

@LRossman
Copy link
Collaborator Author

Valid question @eladsal . Since I thought there was some concern about "API function creep" I limited the vertex functions to the minimum needed to display and edit a network map in a GUI. I don't think that adding the functions EN_getvertices (to retrieve all vertex coordinates at once) and EN_setvertex (to replace an existing vertex's coordinates) helps much since:

  • After all vertices are retrieved in an array by EN_getvertices they would likely be placed in a list structure by the GUI so that efficient editing could occur. It seems simpler to just build the list directly by successive calls to EN_getvertex.
  • After editing, EN_setvertex can't be used to replace existing vertices in the Toolkit database since the number and ordering of vertices may have changed.

These two functions can always be added later if we see a demand for them, but for now I think we have all we need to allow the Toolkit to function as the back end of a GUI.

@eladsal
Copy link
Member
eladsal commented Oct 30, 2019

@LRossman, I understand.

I have tested the functions and I think we are good to go. Thanks!

@Mariosmsk
Copy link
Member

Thanks, @LRossman! Maybe it's good to rename the function ENsetvertices to ENsetvertex? like ENgetvertex, ENgetvertexcount.

I also tested and looks ok!

#151

@samhatchett
Copy link
Member

I think the function names are appropriate for what they do. This also avoids potential confusion over allocation/cleanup responsibility.

@LRossman LRossman merged commit 271d855 into dev Oct 31, 2019
@LRossman LRossman deleted the lrossman-vertices branch November 9, 2019 14:27
@rjanke20 rjanke20 mentioned this pull request Dec 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LRossman @eladsal @Mariosmsk @samhatchett
0