Conversation
| while (result != null) { | ||
| log.info("-- JSON size: " + result.getSecond().length()); | ||
|
|
||
| Bin b = new Bin(); |
There was a problem hiding this comment.
Please don't use single character names
There was a problem hiding this comment.
You're being pedantic, the code is crystal-clear, but I will fix it for you. Would Bin bin be more appropriate?
| b.setData(result.getSecond()); | ||
| mergedViz.addBin(b); | ||
|
|
||
| result = analyzer.processNextTimeBin(); |
There was a problem hiding this comment.
Hm, we can leave it like this for now. But having a while loop with a null condition looks error prone. Maybe the Iterator Interface is what you want here? I'm not sure, but maybe that would come close to the yield return we were talking about.
There was a problem hiding this comment.
This is the main thing I was wondering about in this commit. It feels dirty to me as well. In Java I'm not sure what the least-errorprone / most-appropriate form would be. I'll look at the Iterator interface.
| @Lob | ||
| private String data = ""; | ||
| @OneToMany(mappedBy = "visualization", fetch = FetchType.LAZY, orphanRemoval = true, cascade = CascadeType.ALL) | ||
| Set<Bin> bins = new HashSet(); |
There was a problem hiding this comment.
The bins property should be private.
| } | ||
|
|
||
| @GET | ||
| @Path("/bins") |
There was a problem hiding this comment.
Maybe it would be a good idea to rename the endpoints.
data -> bin
bins -> startTimes
| return startTimes; | ||
| } | ||
|
|
||
| private Visualization findVisualization(Agent agent, String vizId) { |
There was a problem hiding this comment.
is this method still used? otherwise please remove.
Janekdererste
left a comment
Janekdererste
left a comment
There was a problem hiding this comment.
Please make changes and merge
3 participants
Seems to all be working and the Jersey code is WAAAAY better than what I had before.