DateRangeToStringConverter #109
Conversation
| * @param {Object} [options] - An object with formatting options for Intl.DateTimeFormat | ||
| * @returns {string} The formatted date range string | ||
| */ | ||
| convert: { |
There was a problem hiding this comment.
This wasn't my guidance: converters only take one argument in the convert method, and since it's a DateRangeToStringConverter, the argument passed to convert has to be a Range instance. locales and options needs to be properties of the converter itself
maxwell-jordan
force-pushed
the
feature/date-range-to-string-converter
branch
from
c22d8b0 to
a4c9bc0
Compare
| * @param {Object} [options] - An object with formatting options for Intl.DateTimeFormat | ||
| */ | ||
| constructor: { | ||
| value: function DateRangeToStringConverter(locales, options) { |
There was a problem hiding this comment.
It's better to avoid having constructors with arguments as much as possible. They can't be used with serialization/deserialization anyway, when subclassing, one has to remember to call it with the expected arguments. Their main benefit is conciseness when used in code / imperatively
| const dateTimeFormat = new Intl.DateTimeFormat(locales, options); | ||
| return dateTimeFormat.formatRange(startDate, endDate); | ||
| value: function (range) { | ||
| const dateTimeFormat = new Intl.DateTimeFormat(this.locales, this.options); |
There was a problem hiding this comment.
There is no reason to create that object every time convert() is called, is there ?! Please refactor so the converter has only one it reuses.
2 participants
adds DateRangeToStringConverter which takes (startDate, endDate, locales, options) as parameters to output a stringified date in the specified format