ICU-10752 Spread (s|g)etRelativeYear to subclass

[FrankYFTang]

Remove the switch statment implementaiton in Calendar::(g|s)etRelatedYear and move the code into each subclass as proper OOP style.

Checklist

• [X] Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-10752
• [X] Required: The PR title must be prefixed with a JIRA Issue number.
• [X] Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
• [X] Required: Each commit message must be prefixed with a JIRA Issue number.
• [X] Issue accepted (done by Technical Committee after discussion)
• [ ] Tests included, if applicable
• [ ] API docs and/or User Guide docs changed or added, if applicable

[FrankYFTang]

I don't think we need to rush for 72, we can wait till 73.

[pedberg-icu]

I don't think we need to rush for 72, we can wait till 73.

Well, this must wait until 73, since it is after the 72 code freeze. So it cannot be merged until after the maintenance branch is forked for 72. I will wait to review until then.

[markusicu]

@FrankYFTang looks ready to merge for ICU 73.

[jira-pull-request-webhook[bot]]

Notice: the branch changed across the force-push!

• icu4c/source/i18n/calendar.cpp is different
• icu4c/source/i18n/chnsecal.cpp is different
• icu4c/source/i18n/chnsecal.h is different
• icu4c/source/i18n/hebrwcal.cpp is different
• icu4c/source/i18n/hebrwcal.h is different
• icu4c/source/i18n/indiancal.cpp is different
• icu4c/source/i18n/indiancal.h is different
• icu4c/source/i18n/islamcal.cpp is different
• icu4c/source/i18n/islamcal.h is different
• icu4c/source/i18n/persncal.cpp is different
• icu4c/source/i18n/persncal.h is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[FrankYFTang]

Or are these existing constraints that prevent that?

Introducing a getRelatedYearOffset() function means I will need to make an API change, since that will mandate all the subclassed to add a new method, right? (consider those subclass of Clander which is not shipped with ICU.

[richgillam]

Introducing a getRelatedYearOffset() function means I will need to make an API change, since that will mandate all the subclassed to add a new method, right?

I didn't really think about that. But no, not necessarily. I'd think you could introduce that new method as an internal method that we use without opening it up to anyone else. You couldn't make it a pure virtual method, of course-- you'd have to have an implementation at the top level that either did something trivial (like return 0) or threw an exception.

Of course, if the geRelatedYear() method is currently declared as pure virtual at the top level of Calendar, changing it to have an implementation might have negative API-compatibility effects; I'm not completely sure about this.

But you merged, so it's all moot. I don't think this is worth filing a new ticket over or asking you to do more work.