Conversation
eeverman
left a comment
eeverman
left a comment
There was a problem hiding this comment.
Thanks for taking this one one - Nice progress!
|
|
||
| /** | ||
| * A Property that refers to a Long value. | ||
| * A Property that refers to a LocalDateTime value. |
There was a problem hiding this comment.
Nice catch - I wonder how long that has been there??
| * | ||
| * By default this uses the TrimToNullTrimmer, which removes all whitespace from | ||
| * the value and ultimately null if the value is all whitespace. The String | ||
| * constructor version is used when creating instances of BigDecimal. |
There was a problem hiding this comment.
The note about 'BigDecimal' can just be deleted. But, it would be nice to note how the string value is interpreted in this class. There is a note about string date format further down.
| * | ||
| * @author chace86 | ||
| */ | ||
| public class ZonedDateTimeProp extends PropertyBase<ZonedDateTime> { |
There was a problem hiding this comment.
It looks like you started from BigDecimal as a template for this class, but LocalDateTime might be a better place to start. The builder methods in LocalDateTime use time related terms like before and after, rather that greater than or less than.
| * @param reference value the property must be greater than | ||
| * @return the builder instance | ||
| */ | ||
| public ZonedDateTimeBuilder mustBeGreaterThan(ZonedDateTime reference) { |
There was a problem hiding this comment.
See naming in LocalDateTime for all.
| * Extended by nested static classes. The nested classes implement | ||
| * constraints that may be used when building the property. | ||
| */ | ||
| public abstract class ZonedDateTimeValidator implements Validator<ZonedDateTime> { |
There was a problem hiding this comment.
Similar, refer to LocalDateTimeValidator.java for method names, ie before/after.
| try { | ||
| return ZonedDateTime.parse(sourceValue); | ||
| } catch (Exception e) { | ||
| throw new ParsingException("Unable to convert to a LocalDateTime", sourceValue, e); |
|
|
||
| if (sourceValue != null) { | ||
| try { | ||
| return ZonedDateTime.parse(sourceValue); |
There was a problem hiding this comment.
I think we want to make this easier to construct. The default parse method requires the square bracket construction where as most users don't care about the named region, just the hour offset. Can we switch to this format/parser: https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html#ISO_ZONED_DATE_TIME
That should allow either.
|
Hey @chace86 - Do you have any interest in picking this back up? FYI - The naming conventions have changed a bit for the builder methods: The 'must' prefix has been removed. Compare to the LocalDateTimeProp (the non-deprecated methods) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #477 +/- ##
==========================================
- Coverage 78.56% 78.23% -0.33%
==========================================
Files 122 125 +3
Lines 3643 3690 +47
Branches 585 591 +6
==========================================
+ Hits 2862 2887 +25
- Misses 603 625 +22
Partials 178 178 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
WIP! Working on tests for property.