ICU-22994 Use C++ typeid() instead of Calendar::getType() for calendar type comparison

[FrankYFTang]

Use typeid for calendar type comparison instead of strcmp.

Checklist

• Required: Issue filed: ICU-22994
• Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
• Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
• Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/i18n/datefmt.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/i18n/smpdtfmt.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/i18n/windtfmt.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[markusicu]

Questions that I had asked on the ticket:

• Why is using getType() bad? It seems like it’s public and advertised.
• Is strcmp() actually slow for short strings that usually differ in the first couple of characters?

[FrankYFTang]

I think such usage is bad because our code could easily misspell the string

[markusicu]

nit: commit message typo "misuage"; i just fixed the PR title

[FrankYFTang]

PTAL

[richgillam]

This all looks fine. No notes beyond those the other reviewers have already put out.

That said, it looks like all of this is calendar-specific logic in the date formatters-- that is, lots of "if this is an XXX calendar, do this special thing we only do for XXX calendars". It's clearly been that way for a long time, and I'm certainly not proposing you redesign the whole thing now, but I wonder (longer term) if we can find a way to eliminate all of that in favor of better compartmentalization between classes and more actual polymorphic behavior. I don't know what that'd look like or if it's really worth the trouble, but it really seems like the formatters have to know way too much about the internals of the calendars right now.

[FrankYFTang]

This all looks fine. No notes beyond those the other reviewers have already put out.

That said, it looks like all of this is calendar-specific logic in the date formatters-- that is, lots of "if this is an XXX calendar, do this special thing we only do for XXX calendars". It's clearly been that way for a long time, and I'm certainly not proposing you redesign the whole thing now, but I wonder (longer term) if we can find a way to eliminate all of that in favor of better compartmentalization between classes and more actual polymorphic behavior. I don't know what that'd look like or if it's really worth the trouble, but it really seems like the formatters have to know way too much about the internals of the calendars right now.

I totally agree with you. It is my desire to do that later but I still have trouble to figure out how to achieve that.

[richgillam]

I totally agree with you. It is my desire to do that later but I still have trouble to figure out how to achieve that.

Yeah, it's anything but straightforward. We'll figure something out eventually, I'm sure.

[FrankYFTang]

one issue filed under https://unicode-org.atlassian.net/browse/ICU-23030

ftang made the changes that roubert requested

[markusicu]

I dismissed @roubert 's request for changes since @FrankYFTang made all of those changes.

Please squash, and watch for commit message typos.

[jira-pull-request-webhook]

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

[roubert]

LGTM