pull request in draft #10
Conversation
|
Can you please remove all unnecessary files from the commit? Meaning those like *.DS files. Can you also please merge enhsp-20 backend into your fork and solve conflicts? Usually you don't want to have open conflicts at the time of the pull request |
|
Can you also add a few lines into the README.md file explaining how to operate the library in order to have it working with post-hoc visualisation. |
hstairs
left a comment
hstairs
left a comment
There was a problem hiding this comment.
Try to address all comments, and then I'll do a final check on Monday (7 of July)
| private final int id; | ||
| private final Integer parentId; | ||
|
|
||
| private static SearchEventLogger eventLogger; |
There was a problem hiding this comment.
The choice to have a private static event logger is not great. Are we sure this needs to be a property of SearchNode? I understand you need a global access because you have many Search Nodes but this is not a good design solution. I think there is a way to handle the logging information externally by taking information written in the Search Node to analyse.
There was a problem hiding this comment.
The static property eventLogger had been removed in the "Fix required" commit
| public Object[] helpfulActions; | ||
|
|
||
| public SearchNode (State s1, Object action, SearchNode father, float g, float h) { | ||
| private static int NEXT_ID = 0; |
There was a problem hiding this comment.
This is connected to the EventLogger problem. Please try to limit as much as possible the use of static variables
There was a problem hiding this comment.
We tried to remove the static variable NEXT_ID from SearchNode but we didn't find a possible solution. Also increasing the parent-id during the generation of the son is not correct because all "brothers" would have the same id.
There was a problem hiding this comment.
It seems you are completely changing the algorithm here. Is this wanted? What are you doing here? @SaraBe01
There was a problem hiding this comment.
What are you doing here? Again, this seems like a complete change of the algorithm. Please explain. @SaraBe01
| LinkedList plan = new LinkedList<>(); | ||
| lastState = c.s; | ||
| while (c.transition != null) { | ||
| if (c.transition != null) {//this is an action |
There was a problem hiding this comment.
We didn't remove the line of code but only the comment, so we added it again
| } | ||
|
|
||
| public void initHandle(SearchNode init){ | ||
| searchSpaceHandle = init;//this needs to have an handle on the initial state for saving it into a json file |
There was a problem hiding this comment.
We didn't remove the line of code but only the comment, so we added it again
|
|
||
| public int numberOfSons; | ||
| public float minSoFar; | ||
| private static int nextId = 0; //only way tohav a global ID for all nodes |
| @@ -1,208 +1,175 @@ | |||
| /* | |||
There was a problem hiding this comment.
Why are you removing the credits?
3 participants
Inizio lavori progetto Berselli e Ferrari