-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix number truthiness by casting to individual types #857
Conversation
if (object instanceof Integer) { | ||
return (Integer) object != 0; | ||
} | ||
if (object instanceof Double) { | ||
return (Double) object != 0; | ||
} | ||
if (object instanceof Long) { | ||
return (Long) object != 0; | ||
} | ||
if (object instanceof Short) { | ||
return (Short) object != 0; | ||
} | ||
return (Float) object != 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, I could just do:
return ((Number)object).doubleValue() != 0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I don't think that will quite work - Python doesn't treat 0.0
as truthy
My mistake, it does! This is shorter and more to the point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I wrote it like that thinking it would be faster, but I tested it. This shorter way is faster for longs, about the same time for floats and ints so I'm going to do the simpler approach
@@ -39,7 +39,19 @@ public static boolean evaluate(Object object) { | |||
} | |||
|
|||
if (object instanceof Number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are more subclasses of Number
. While you don't have to handle them all, perhaps a sane default would be to use Number.floatValue()
to avoid a potential bad cast to Float
.
@@ -15,6 +14,46 @@ public void itEvaluatesObjectsWithObjectTruthValue() { | |||
.isFalse(); | |||
} | |||
|
|||
@Test | |||
public void itEvaluatesIntegers() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth a utility function here that takes a and b and does the assertions so you're not duplicating the code for every type.
Truthiness for non-integer numbers functions differently in Jinjava than it does in Jinja. The following should print
yes
, but since it currently only takes theNumber.intvalue()
, it would return false. This PR fixes the issue by checking which type of Number the object is.