Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better rules to decide between 19 and 20 for parsing YY #1439

Closed
Kiwironic opened this issue Jan 23, 2014 · 7 comments · Fixed by #1545
Closed

Better rules to decide between 19 and 20 for parsing YY #1439

Kiwironic opened this issue Jan 23, 2014 · 7 comments · Fixed by #1545

Comments

@Kiwironic
Copy link

Currently the rule for parsing YY is
If greater than 68 will return 1900's, otherwise 2000's.

datePartArray[YEAR] = toInt(input) + (toInt(input) > 68 ? 1900 : 2000); 

For many projects such as the one I am working on, this rule is not valid. I am using this rule:
If entered YY is greater than current year (YY), it will return 1900's, otherwise 2000's.

datePartArray[YEAR] = toInt(input) + 
(toInt(input) > (moment(moment()).format("YY")) ? 1900 : 2000);

It's a good idea to expose this value (68) to be configurable.
What do you think?

@icambron
Copy link
Member

I'm skeptical. Your use case sounds very esoteric, and AFAIK this configuration has never been requested before. Without knowing more about what you're doing, it seems like it would make more sense just to a bit of your own parsing. I guess I need to be convinced this is a problem a significant number of people will face.

@ichernev
Copy link
Contributor

I agree with @icambron here. Maybe we can extract the logic into a function, so you can overwrite it with your own, but this can break miserably in a bigger application.

Another solution is to make format tokens extensible, so one can provide non-strict, strict regexes, parsing logic and formatting logic.

@RAlfoeldi
Copy link

I'm surprised nobody else has requested this before. Basically any birthday entry is going to have problems with 1968 as cutoff date.

I'm allowing users to enter (birth-)dates as DDMMYY, DDMMYYYY, DD.MM.YY, DD.MM.YYYY and LL (business app, keyboard oriented efficient data entry). 01.01.50 will almost always mean 01.01.1950.

The same app (insurance) allows entries in the future, so a 01.01.20 could be assumed to be 01.01.2020.

The non plus ultra solution would be to extend moment(string, dateFormats) with a 'cutoff data param, so that you could define the date per call.

Fine would be exposing the cutoff year. (Don't get me wrong: moment rocks! :-)

As @icambron said, the do it yourself solution would include a lot of parsing, basically reimplementing (copying) most parts of the moment parse routine.

@Kiwironic
Copy link
Author

Hi,

I am a professional developer, and working on a massive Government project. This rule is not a suggestion, it is a business rule by the BA's and I had to implement it, so if you are trying to find out if there is a need for it. Yes, there is a real need for it.

I have already solved the problem by changing the value in the library, although it is not as maintainable as I would like it to be, but it works fine with the least amount of work done (as you may know, some Govt projects have really tight budgets).

I have suggested to have this value changed to be configurable which is a reasonable suggestion as having a hard-coded value in the library is not the best it could be.

I trust your judgement and would leave this to you. Moment is a great library and my suggestion is an honest attempt to help it get closer to perfection (as I have mentioned before my issue has been solved). So I will leave this for you to take care of.

All the Best,

Cheers,
Mena

@icambron
Copy link
Member

I've submitted a pull request (#1545) that allows external customization of how two digit years are parsed. You can see an example of how to do use it in the diffs for the create.js test. If @ichernev decides to pull it, it'll be in the next version.

@Kiwironic
Copy link
Author

Awesome, Thanks a lot

@RAlfoeldi
Copy link

@icambron, @ichernev: Thanx! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants