Move date parsing logic to DateParser #189
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recently I implemented DateValue function that would parse dates in different formats.
Since then, I noticed that there are other functions (YEARFRAC, in particular) that use date parsing logic and that they are using a class DateParser. This brought my attention, because DateParser is unable to parse dates in other formats, like 'yyyy-MM-dd'.
I think it would make sense to use the same logic for date parsing in all places, so I moved my code from DateValue function to DateParser. Since both DateParser and DateValue tests are still green without any changes, I think this refactor is valid.
I also moved DateParser from atp package to util, since it is used outside atp now - although I see that it might raise some flags, since it is a public class and theoretically it might break compatibility for other projects that might be using DateParser - please advice, whether it is acceptable to move this class or should it be left in current package.