OWLDiff Web #5
OWLDiff Web #5
Conversation
There was a problem hiding this comment.
@luvave see comments. Main points are:
- almost no documentation - the code needs to be documented (public classes/methods) to be reusable to others
- improve testing (service/controller)
- look at the split of the service/controller layer - currently the separation is not well done
| <dependency> | ||
| <groupId>com.github.galigator.openllet</groupId> | ||
| <artifactId>openllet-jena</artifactId> | ||
| <version>2.6.5</version> |
There was a problem hiding this comment.
A variable would be helpful.
|
|
||
| private List<NodeModelDto> children; | ||
|
|
||
| @JsonIgnore |
There was a problem hiding this comment.
Why this?
In general, the code lacks documentation - to make it understandable at least interfaces and public methods should be well documented
| import org.semanticweb.owlapi.model.OWLDocumentFormat; | ||
|
|
||
| public enum OWLDocumentFormatEnum { | ||
| //TODO: Add more formats? Source http://robot.obolibrary.org/merge |
There was a problem hiding this comment.
It is much better to record TODOs as individual issues instead of having them in text comments.
|
|
||
| private OWLDocumentFormat format; | ||
| private String extension; | ||
| private OWLDocumentFormatEnum(OWLDocumentFormat format, String extension){ |
There was a problem hiding this comment.
Consider using some formatting library - Idea default formatted (Ctrl+Alt+L), or Checkstyle .
E.g. the constructor should be separated with a blank line from the fields
|
|
||
| @GetMapping | ||
| public String ping() { | ||
| return "pong"; |
There was a problem hiding this comment.
Spring boot has a built-in support for /health endpoint - https://docs.spring.io/spring-boot/docs/1.3.5.RELEASE/reference/html/production-ready-monitoring.html
|
|
||
| public String getRet() { | ||
| String tmp = this.ret; | ||
| this.ret = null; |
There was a problem hiding this comment.
This looks more like 'poll' than 'get'...
| @@ -0,0 +1,60 @@ | |||
| package cz.cvut.kbss.owldiff.api; | |||
There was a problem hiding this comment.
Both controller logic and service logic should be tested.
| onAgree: (val?: any) => void, | ||
| } | ||
|
|
||
| export const ConfirmDialog = (props: ConfirmDialogProps) => { |
There was a problem hiding this comment.
"{ return" could be ommited
| return ( | ||
| <TreeItem | ||
| key={treeItemData.id} | ||
| sx={props.colorSettings ? |
There was a problem hiding this comment.
would deserve a dedicated variable/computation upfront - it is hardly readable
| @@ -0,0 +1,75 @@ | |||
| # | |||
There was a problem hiding this comment.
Could not the translation be partially reused with API?
No description provided.