Regarding the DateTime parameter in Parse and ToShortString about activelogin.identity HOT 4 CLOSED

Let me elaborate on the issue.

I find this part of the api confusing, that is why I created the issue.
Most users are probably not aware of why this matters, or do not need the feature.

The usecase for providing a date when parsing an identity number I can think of is that you are parsing old data: The user entered the data with a hyphen when they were 99 years old and have since turned 100 which means the parsing will fail when performed at a later date. An edge case, but I can understand that we want to support this for completeness.

But the usecase for providing a date when calling ToString/ToShortString I do not really see. What is it I want to do at that point? “What would the string representation of this identity number be if I asked for it before this person turned 100”? Who wants to do that?

So I guess what I would like to propose is that if we consider the Parsing usecase to be important than we remove the overload without date and force the users of the api to always call it with a date. That way we make the users aware of the potential issue with people turning 100 after the string has been entered.
I think I prefer this solution the most actually. Let the users of the api create their own convenience method for calling our api with date = DateTime.UtcNow, so we don't hide the issue.

An alternative solution would be to not have this as an overloaded method, instead we name the method something else: ParseWithCustomDelimiterCutOffDate, or something shorter 😉

In any case we need to add some tests for this usecase.

For ToString/ToShortString I think we could do the same thing but since I think that usecase is weird, right now I would propose instead simply removing the overload that takes a date.

from activelogin.identity.

After some offline discussions we have decided to:

• Keep the data parameter both for .To10DigitString() and .Parse()
• Rename the parameter to make it more clear what it does, not it's only named date

Suggestions of possible names?

from activelogin.identity.

Since only the year part matters maybe it makes sense to call the parameter

• currentYear

That's sort of what it is, right? CurrentYear for the execution of the method call, so to speak.

Otherwise

• decisionDate
• delimiterDecisionDate

from activelogin.identity.

Since only the year part matters maybe it makes sense to call the parameter

• currentYear

I like this! I've just created a PR.

from activelogin.identity.