Add generic Value#convert(Object) method #7076
Conversation
| * | ||
| * @param object the object to convert | ||
| * @return the equivalent {@link Value} | ||
| * @throws IllegalArgumentException if not able to convert the object to {@link Value} |
There was a problem hiding this comment.
Might want to throw a checked exception to more forcibly signal to callers that they need to handle this.
There was a problem hiding this comment.
Decided against this:
- This method is analagous to jackson ObjectMapper#convertValue, which only throws a runtime exception. This API is quite popular and I've used it myself without issue for years, suggesting that a runtime exception is sufficient.
- I'm not sure what checked exception would be most appropriate to throw. Probably some subclass of
IOException, and we'd probably want to introduce our own dedicated exception. In contrast,IllegalArgumentExceptionpretty perfectly describes the error and seems appropriate to throw.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7076 +/- ##
============================================
+ Coverage 90.06% 90.10% +0.03%
- Complexity 7320 7335 +15
============================================
Files 825 825
Lines 22048 22081 +33
Branches 2179 2188 +9
============================================
+ Hits 19857 19895 +38
+ Misses 1511 1505 -6
- Partials 680 681 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…y-java into value-convert-object
jkwatson
left a comment
jkwatson
left a comment
There was a problem hiding this comment.
seems ok to me. Needs a rebase to get the apidiffs up to date, when you get a moment.
|
I'd suggest holding on this for a bit longer while we figure out the impact of extending attributes to support any value, just in case. |
should we move this to a Draft, then? |
…y-java into value-convert-object
|
Marking "Ready for review" now that #7814 is merged. |
| for (Object entry : list) { | ||
| valueList.add(Value.convert(entry)); | ||
| } | ||
| return Value.of(Collections.unmodifiableList(valueList)); |
There was a problem hiding this comment.
Unmodifiable wrapper probably not needed since Value is already immutable?
| } | ||
| valueMap.put((String) key, Value.convert(value)); | ||
| }); | ||
| return Value.of(Collections.unmodifiableMap(valueMap)); |
4 participants
No description provided.