Issue 10382171: Make most arguments to Date constructor optional.

Issue 10382171: Make most arguments to Date constructor optional. (Closed)

Created:
8 years, 7 months ago by floitsch

Modified:
8 years, 7 months ago

Reviewers:
sorinmocanu, ngeoffray

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Make most arguments to Date constructor optional. Fixes issue 2582. Committed: https://code.google.com/p/dart/source/detail?r=7652

Patch Set 2 : Address comments. #

Created: 8 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+106 lines, -24 lines)			Patch
M	corelib/src/date.dart	View	1	1 chunk	+8 lines, -6 lines	0 comments	Download
M	frog/lib/date_implementation.dart	View		1 chunk	+6 lines, -6 lines	0 comments	Download
M	lib/compiler/implementation/lib/mockimpl.dart	View		1 chunk	+6 lines, -6 lines	0 comments	Download
M	runtime/lib/date.dart	View	1	1 chunk	+6 lines, -6 lines	0 comments	Download
A	tests/corelib/date_time5_test.dart	View		1 chunk	+80 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

ngeoffray

LGTM https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart File corelib/src/date.dart (right): https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart#newcode40 corelib/src/date.dart:40: // our tools don't yet. Eventually we want ...

8 years, 7 months ago (2012-05-15 12:17:35 UTC) #2

sorinmocanu

https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart File corelib/src/date.dart (left): https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart#oldcode40 corelib/src/date.dart:40: int month, Where will you document the possible combinations ...

8 years, 7 months ago (2012-05-15 12:21:59 UTC) #3

floitsch

8 years, 7 months ago (2012-05-15 12:34:21 UTC) #4

https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart
File corelib/src/date.dart (left):

https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart#...
corelib/src/date.dart:40: int month,
On 2012/05/15 12:21:59, sorinmocanu wrote:
> Where will you document the possible combinations of arguments which are
> possible?
> Crazy example, should a date implementation support year + hours?

Each argument has a default value. 1 for month and day, and 0 for the others.
Each can be passed as named argument without affecting the others.
So yes: new Date(2012, milliseconds: 499) is legal (although probably not very
common).

https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart
File corelib/src/date.dart (right):

https://chromiumcodereview.appspot.com/10382171/diff/1/corelib/src/date.dart#...
corelib/src/date.dart:40: // our tools don't yet. Eventually we want to have
default values here.
On 2012/05/15 12:17:35, ngeoffray wrote:
> our our -> our

Done.

https://chromiumcodereview.appspot.com/10382171/diff/1/runtime/lib/date.dart
File runtime/lib/date.dart (left):

https://chromiumcodereview.appspot.com/10382171/diff/1/runtime/lib/date.dart#...
runtime/lib/date.dart:116: int hashCode() => value;
On 2012/05/15 12:17:35, ngeoffray wrote:
> bad mergE?

Done.

https://chromiumcodereview.appspot.com/10382171/diff/1/runtime/lib/date.dart#...
runtime/lib/date.dart:212: if (n >= 10) return "0${n}";
On 2012/05/15 12:17:35, ngeoffray wrote:
> ditto?

Done.

Expand Messages | Collapse Messages