Implement reverse operator #20
Conversation
|
To use auto-refresh, install the Chrome extension LiveReload. |
Zackhardtoname
changed the title
Zackhardtoname
force-pushed
the
reverse-operator
branch
from
653637e
to
73dee64
Compare
src/main/java/knox/spring/data/neo4j/controller/KnoxController.java
Outdated
Show resolved
Hide resolved
src/main/resources/static/js/knox.js
Outdated
| @@ -557,6 +557,10 @@ $('#apply-operators-tooltip').click(() => { | |||
| case endpoint.operators.MERGE: | |||
| endpoint.designSpaceMerge(inputSpaces, outputSpace, tolerance); | |||
| break; | |||
|
|
|||
| case endpoint.operators.REVERSE: | |||
| endpoint.designSpaceReverse(inputSpaces, outputSpace); | |||
There was a problem hiding this comment.
see comment in endpoints.js
src/main/java/knox/spring/data/neo4j/controller/KnoxController.java
Outdated
Show resolved
Hide resolved
| try { | ||
| long startTime = System.nanoTime(); | ||
|
|
||
| if (outputSpaceID == null) { |
There was a problem hiding this comment.
We should follow the conventions set by the other functions and throw error if outputSpaceID is null.
There was a problem hiding this comment.
If the output space is null, in the file DesignSpaceService.java, the functions usually choose the first element in the inputBranches as the output space. I think my charges here are consistent?
There was a problem hiding this comment.
I see that in the DesignSpaceService.java, but when you test on the UI, it will throw an error if the outputBranch is empty. But the change you made, where you put get .get(0) in knox.js, is inconsistent with the rest of the repo.
|
I just realized there's nothing on the UI to test this? There should be a new element added to the Operators dropdown to actually use this functionality. |
The reverse operator is actually in the dropdown. |
|
Oh I see it now, I was on the wrong branch. I just tested this and it threw the error
Please test all your changes before making a commit. |
src/main/resources/static/js/knox.js
Outdated
| @@ -557,6 +557,10 @@ $('#apply-operators-tooltip').click(() => { | |||
| case endpoint.operators.MERGE: | |||
| endpoint.designSpaceMerge(inputSpaces, outputSpace, tolerance); | |||
| break; | |||
|
|
|||
| case endpoint.operators.REVERSE: | |||
| endpoint.designSpaceReverse(inputSpaces.get(0), outputSpace); | |||
There was a problem hiding this comment.
this is not valid javascript syntax
There was a problem hiding this comment.
Oh I see it now, I was on the wrong branch. I just tested this and it threw the error
Please test all your changes before making a commit.
Sorry! I was only looking at the output in the terminal. I will remember to check the webpage console as well.
| @@ -559,7 +559,7 @@ $('#apply-operators-tooltip').click(() => { | |||
| break; | |||
|
|
|||
| case endpoint.operators.REVERSE: | |||
| endpoint.designSpaceReverse(inputSpaces.get(0), outputSpace); | |||
| endpoint.designSpaceReverse(inputSpaces[0], outputSpace); | |||
There was a problem hiding this comment.
Zack, you missed my other comment that this is inconsistent with the other functions. This code should be in the Java file, not here. Please squash these commits when you've fixed them.
Functionality: reverse the edges from the input design space. Details: Upon selecting reverse operator a. Delete input option b. Display some guidance Close CIDARLAB#14
Zackhardtoname
force-pushed
the
reverse-operator
branch
from
bec4a3d
to
0e3ca78
Compare
Suggest to close #19
Note that the other examples in the example directory probably should be corrected as well.