Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

fix(DateFilter): fix a wrong type #579

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions lib/filter/date.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ part of angular.filter;
*
*/
@NgFilter(name:'date')
class DateFilter {
static Map<String, String> MAP = {
class DateFilter implements Function {
static final _MAP = const <String, String> {
'medium': 'MMM d, y h:mm:ss a',
'short': 'M/d/yy h:mm a',
'fullDate': 'EEEE, MMMM d, y',
Expand All @@ -33,7 +33,7 @@ class DateFilter {
'shortTime': 'h:mm a',
};

Map<num, NumberFormat> nfs = new Map<num, NumberFormat>();
var _dfs = <String, DateFormat>{};

/**
* [date]: Date to format either as Date object, milliseconds
Expand All @@ -47,18 +47,19 @@ class DateFilter {
* mediumDate is used
*
*/
call(date, [format = r'mediumDate']) {
dynamic call(Object date, [String format = 'mediumDate']) {
if (date == '' || date == null) return date;
if (date is String) date = DateTime.parse(date);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the main public service of this class is delivered through the call() method, it should have type annotations. I would suggest

String call(dynamic date, [String format = r'mediumDate'])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I'm all for (2)
  • I think dynamic is useless in your latest comment,
  • You must be right for dfs (i.e., the other less likely option would be extensibility by inheritance)
  • I think there are lots of place where the _ is missing for private variables...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use of dynamic is no more, no less useless than use of, say, @overrides. (But again, this discussion is for another time.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the r (raw) qualifier from the format default. In this case, it is truly useless :).

if (date is num) date = new DateTime.fromMillisecondsSinceEpoch(date);
if (date is! DateTime) return date;
var nf = nfs[format];
if (nf == null) {
if (MAP.containsKey(format)) {
format = MAP[format];
var df = _dfs[format];
if (df == null) {
if (_MAP.containsKey(format)) {
format = _MAP[format];
}
nf = new DateFormat(format);
df = new DateFormat(format);
_dfs[format] = df;
}
return nf.format(date);
return df.format(date);
}
}
12 changes: 9 additions & 3 deletions test/filter/date_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ library date_spec;
import '../_specs.dart';

main() => describe('date', () {
var morning = DateTime.parse('2010-09-03T07:05:08.008Z'); //7am
var noon = DateTime.parse('2010-09-03T12:05:08.012Z'); //12pm
var midnight = DateTime.parse('2010-09-03T12:05:08.123Z'); //12am
var morning = DateTime.parse('2010-09-03T07:05:08.008Z'); //7am
var noon = DateTime.parse('2010-09-03T12:05:08.012Z'); //12pm
var midnight = DateTime.parse('2010-09-03T12:05:08.123Z'); //12am
var earlyDate = DateTime.parse('0001-09-03T05:05:08.000Z');

var date;
Expand Down Expand Up @@ -57,4 +57,10 @@ main() => describe('date', () {
expect(date(noon, "shortTime")).
toEqual('12:05 PM');
});

it('should use cache without any error', () {

date(noon, "shortTime");
date(noon, "shortTime");
});
});