[LANG-1637] Fix 2 digit week year formatting#688
Conversation
| public void testWeekYear() { | ||
| final GregorianCalendar cal = new GregorianCalendar(2020, 12, 31, 0, 0, 0); | ||
| final DatePrinter printer4Digits = getInstance("YYYY"); | ||
| final DatePrinter printer2Digits = getInstance("YY"); |
There was a problem hiding this comment.
Hi,
Thank you for your PR.
What about "YYY"?
There was a problem hiding this comment.
Please add code in your new test for Y and YYY.
|
@garydgregory can you explain why you closed this PR ? |
|
Reopening, I was going by the "invalid" comment. |
|
@michael-o The PR seems OK to me now. WDYT? |
garydgregory
changed the title
|
Will have a look. |
| @Override | ||
| public final void appendTo(final Appendable buffer, final int value) throws IOException { | ||
| appendDigits(buffer, value); | ||
| appendDigits(buffer, value % 100); |
There was a problem hiding this comment.
I am not convinced by this because it patches the wrong spot.
There was a problem hiding this comment.
the name TwoDigitYearField implies this class encapsulates formatting of year field to a 2 digit string. as such, it seems reasonable for it to have the logic to take any year integer and map it to a decade followed by a year digits.
you can also see that the other method in this class
void appendTo(Appendable buf, Calendar calendar) throws IOException
implementing the Rule interface, does the same
There was a problem hiding this comment.
Hi @michael-o
Earlier you said "I am not convinced by this because it patches the wrong spot." implying there was a "right" spot, but where would that be?
There was a problem hiding this comment.
I think here: https://issues.apache.org/jira/browse/LANG-1637?focusedCommentId=17257625&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17257625 and this method must throw IAE if (value >= 100).
There was a problem hiding this comment.
Well... which way is this one going to go? @ugonen Are you going to update your PR or @michael-o do you want to provide a different PR?
There was a problem hiding this comment.
I think for the ease of use we have to use this one because my solution leaks abstraction. It makes too many assumptions about in the delegated NumberRule
4 participants
https://issues.apache.org/jira/browse/LANG-1637