CLIENT-4533 Change asInt() and asFloat() to always generate casting Exp#68
CLIENT-4533 Change asInt() and asFloat() to always generate casting Exp#68
Conversation
…posability with comparison operators
| private Exp valueToExp() { | ||
| if (value instanceof Boolean b) return Exp.val(b); | ||
| if (value instanceof String s) return Exp.val(s); | ||
| if (value instanceof Float f) return Exp.val(f); | ||
| return Exp.val((Integer) value); | ||
| } |
There was a problem hiding this comment.
Can "value" be other types, like Long, Double, byte[], primitive types, etc? If so, we should make this method more flexible
There was a problem hiding this comment.
Currently no, but this implementation is somewhat fragile, and there will be support for more types in future, so updating.
| private Exp valueToExp() { | ||
| if (value instanceof Boolean b) return Exp.val(b); | ||
| if (value instanceof String s) return Exp.val(s); | ||
| if (value instanceof Float f) return Exp.val(f); | ||
| return Exp.val((Integer) value); | ||
| } |
There was a problem hiding this comment.
Should we make one of these methods static and possibly in a helper class rather than violating the DRY principle?
| case INT -> Exp.Type.FLOAT; | ||
| case FLOAT -> Exp.Type.INT; | ||
| }; | ||
| } | ||
|
|
||
| public static ExprPartsOperation castTypeToOperation(CastType castType) { | ||
| return switch (castType) { | ||
| case INT -> ExprPartsOperation.TO_INT; | ||
| case FLOAT -> ExprPartsOperation.TO_FLOAT; |
There was a problem hiding this comment.
These methods both start with "cast" so I assume they have a similar function. Yet one maps INT -> INT, the other INT -> FLOAT. Is this deliberate or is one of the methods backwards? If so I would add a comment in to explain why.
There was a problem hiding this comment.
This is actually deliberate. The inverse mapping in castSourceExpType exists because when toInt() is used, the bin must be read with Exp.Type.FLOAT (its actual type) before the cast converts it to INT, it can be seen in PathOperandUtils.processValueType():56-59.
You are right that this can be confusing. The name castSourceExpType doesn't immediately convey "inverse of target". Adding a comment.
2 participants
No description provided.