Return object index from EN_addnode and EN_addlink (issue #432) #461
Conversation
Adds an output argument to EN_addnode and EN_addlink that returns the index of the newly added object. Also refactors the validity check on object ID names.
Codecov Report
@@ Coverage Diff @@
## dev #461 +/- ##
==========================================
+ Coverage 70.32% 70.39% +0.06%
==========================================
Files 23 22 -1
Lines 9907 9889 -18
==========================================
- Hits 6967 6961 -6
+ Misses 2940 2928 -12
Continue to review full report at Codecov.
|
tests/test_node.cpp
Outdated
| BOOST_REQUIRE(error == 252); | ||
|
|
||
| error = EN_addnode(ph, (char *)"N\"3", EN_JUNCTION); | ||
| error = EN_addnode(ph, (char *)"N\"3", EN_JUNCTION, &index); |
There was a problem hiding this comment.
Note: This is ok with EPANET UI.
There was a problem hiding this comment.
This observation about the UI accepting N\"3 as a valid ID raises a number of points:
- Not only will the UI accept the ID but so will the
EN_openAPI function that reads in an input file. - The new update to the UI will not allow a space, semi-colon, or double quote to be entered into an ID field in the Property Editor (which is a good thing).
- The engine code's input file reader does not do any checking of ID names. If they exceed 31 characters they are truncated. If the ID in the original input line had a pair of double quotes for the ID name field, the name will be truncated at the first quote and the characters within the quotes will be treated as a separate token. This behavior may (or may not) trigger an input error.
It's my opionion that we stick with the current restriction on valid ID names used with the EN_add functions but also extend it to the names processed through an .INP file, even if this might affect backward compatibility.
|
This is an update on the question of not allowing double quotes in an object ID name. A closer inspection of EPANET's input file parsing routine shows that a double quote in an ID name is only a problem if it is the first character in the name. So |
| @@ -57,7 +57,7 @@ BOOST_FIXTURE_TEST_CASE(test_node_validate_id, FixtureInitClose) | |||
| error = EN_addnode(ph, (char *)"N 3", EN_JUNCTION, &index); | |||
| BOOST_REQUIRE(error == 252); | |||
|
8000 |
|||
| error = EN_addnode(ph, (char *)"N\"3", EN_JUNCTION, &index); | |||
| error = EN_addnode(ph, (char *)"\"N3", EN_JUNCTION, &index); | |||
| BOOST_REQUIRE(error == 252); | |||
There was a problem hiding this comment.
with matlab test:
Returns of this function with this node ID is zero, not 252.
There was a problem hiding this comment.
In C/C++ code, a double quote in a string literal needs to be preceeded by an escape character (the backslash) so it is not interpreted as the end of string marker. So in the C code for test_node.cpp, the literal "\"N3" is read as "N3 by the compiler and fails to be a valid ID name. In Matlab, I'm guessing that '\"N3' is interpreted as \"N3 which is valid since it doesn't begin with a double quote.
There was a problem hiding this comment.
@LRossman , Thanks! Then this PR looks ok.
Adds an output argument to EN_addnode and EN_addlink that returns the index of the newly added object.
Also refactors the validity check on object ID names.