added array support #46
Conversation
gkorland
left a comment
gkorland
left a comment
There was a problem hiding this comment.
@DvirDukhan so why did you mark me to review?
| } | ||
|
|
||
| private List<Object> deserializeArray(Object rawScalarData) { | ||
| List<Object> res = new ArrayList<>(); |
There was a problem hiding this comment.
List<Object> res = new ArrayList<>(array.length());
moved to value type
| @Deprecated | ||
| public ResultSet.ResultSetScalarTypes getType() { | ||
| if (type.ordinal() >= 6 && type.ordinal() < 12){ | ||
| return ResultSet.ResultSetScalarTypes.values()[type.ordinal()-6]; |
There was a problem hiding this comment.
Shouldn't this be like setType, adding 6 to the deprecated types rather than the other way around?
There was a problem hiding this comment.
in setType, you add 6 to the deprecated types and get the valid type
in getType, which is a deprecated call, the caller expects deprecated type.
There was a problem hiding this comment.
Might be better
type.ordinal() > ResultSet.ResultSetScalarTypes.PROPERTY_DOUBLE.ordinal()
jeffreylovitz
left a comment
jeffreylovitz
left a comment
There was a problem hiding this comment.
Are the enum values of ResultSetScalarTypes ever user-accessible? I think that this deprecation logic is likely overkill, as the enum names have changed but the values have merely been extended. Let's discuss this on Monday!
| */ | ||
| public interface ResultSet extends Iterator<Record> { | ||
|
|
||
| @Deprecated |
There was a problem hiding this comment.
you should add JDoc @Deprecated use ... instead
| @Deprecated | ||
| public Property(String name, ResultSet.ResultSetScalarTypes type, Object value) { | ||
| this.name = name; | ||
| this.type = type; |
There was a problem hiding this comment.
don't you just want to convert from ResultSetScalarTypes to ResultSetValueTypes and avoid this second valueType field?
There was a problem hiding this comment.
is this still relevant?
| @Deprecated | ||
| public ResultSet.ResultSetScalarTypes getType() { | ||
| if (type.ordinal() >= 6 && type.ordinal() < 12){ | ||
| return ResultSet.ResultSetScalarTypes.values()[type.ordinal()-6]; |
There was a problem hiding this comment.
Might be better
type.ordinal() > ResultSet.ResultSetScalarTypes.PROPERTY_DOUBLE.ordinal()
src/main/java/com/redislabs/redisgraph/impl/resultset/ResultSetImpl.java
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #46 +/- ##
==========================================
+ Coverage 75.9% 76.15% +0.24%
==========================================
Files 20 20
Lines 469 478 +9
Branches 72 74 +2
==========================================
+ Hits 356 364 +8
Misses 85 85
- Partials 28 29 +1
Continue to review full report at Codecov.
|
DO NOT MERGE YET!!!!!
added support for array