Improving scale transformation service #4439
Conversation
|
@martinvw here is the PR for the scale transformation service |
|
Related to openhab/openhab-docs#486 Thanks! |
| private static final long serialVersionUID = 3860553217028220119L; | ||
| private final HashSet<Object> keys = new LinkedHashSet<Object>(); | ||
|
|
||
| public OrderedProperties() { |
There was a problem hiding this comment.
Consider leaving this constructor away, it will be added by the compiler anyway
| } | ||
| Optional<Range> match = data.keySet().stream().filter(range -> range.contains(value)).findFirst(); | ||
| if (match.isPresent()) { | ||
| return data.get(match.get()); |
There was a problem hiding this comment.
I believe we can also do it like this:
return data.entrySet().stream().
filter(e -> e.getKey().contains(value)).
findFirst().
map(Map.Entry::getValue).
orElseThrow(() -> new TransformationException("No matching range for '" + source + "'"));There was a problem hiding this comment.
Thanks for your review @martinvw . So much left to do on so small changes :) I still have to progress.
|
|
||
| String transformedResponse = processor.transform(evaluationorder, source); | ||
| Assert.assertEquals("first", transformedResponse); | ||
|
|
There was a problem hiding this comment.
Maybe leave out this newline, note that it also is a notice in the (upcoming) release of the static code analysis.
| } | ||
|
|
||
| public Iterable<Object> orderedKeys() { | ||
| return Collections.list(keys()); |
There was a problem hiding this comment.
I think that we can also return keys direct;ly I do not yet see the added value of the current solution:
Set<Object> orderedKeys() {
return keys;
}| @@ -37,28 +41,50 @@ | |||
| /** RegEx to extract a scale definition */ | |||
| private static final Pattern LIMITS_PATTERN = Pattern.compile("(\\[|\\])(.*)\\.\\.(.*)(\\[|\\])"); | |||
|
|
|||
| class OrderedProperties extends Properties { | |||
| private static final long serialVersionUID = 3860553217028220119L; | |||
| private final HashSet<Object> keys = new LinkedHashSet<Object>(); | |||
There was a problem hiding this comment.
Please use the diamond operator, ie replace the second occurrence of the same generic by <>
|
|
||
| @Override | ||
| public Enumeration<Object> keys() { | ||
| return Collections.<Object> enumeration(keys); |
There was a problem hiding this comment.
Please use the diamond operator, ie replace the second occurrence of the same generic by <>
There was a problem hiding this comment.
Diamond operator does not work here. So left it as is.
There was a problem hiding this comment.
Hmm, sorry then Intellij had some other complaint about that line but its not that important :-)
| @@ -37,28 +41,50 @@ | |||
| /** RegEx to extract a scale definition */ | |||
| private static final Pattern LIMITS_PATTERN = Pattern.compile("(\\[|\\])(.*)\\.\\.(.*)(\\[|\\])"); | |||
|
|
|||
| class OrderedProperties extends Properties { | |||
There was a problem hiding this comment.
I believe we should add the author and goal for this inner class as well.
There was a problem hiding this comment.
Should this inner class be static I see no reason to have a reference to the the outer class.
There was a problem hiding this comment.
Should we document that this implementation has limitations, eg iterating over the entryset will not be Ordered, removing elements will cause misery etc.
| @Test | ||
| public void testEvaluationOrder() throws TransformationException { | ||
| // Ensures that only first matching scale as presented in the file is taken in account | ||
| String evaluationorder = "scale/evaluationorder.scale"; |
There was a problem hiding this comment.
Should we name the variable evaluationOrder :-)
to ensure that they match in the same order than the scale definition file as discussed in PR eclipse-archived#486 Corrections following code review Signed-off-by: Gaël L'hopital <glhopital@gmail.com>
clinique
force-pushed
the
Improve_Scale_Transformation
branch
from
d927162
to
47c9013
Compare
|
Thanks! |
to ensure that they match in the same order than the scale definition file as discussed
in PR openhab/openhab-docs#486
Signed-off-by: Gaël L'hopital glhopital@gmail.com