Add DateTime.replace.

Add DateTime.replace.

Allows replacing part of a DateTime without touching the rest.

Fixes issue #24644
BUG= https://github.com/dart-lang/sdk/issues/24644

[Lasse Reichstein Nielsen, 2015-11-23 13:33:17]

Description was changed from

==========
Add DateTime.replace.

Allows replacing part of a DateTime without touching the rest.
==========

to

==========
Add DateTime.replace.

Allows replacing part of a DateTime without touching the rest.

Fixes issue #24644
BUG= https://github.com/dart-lang/sdk/issues/24644
==========

[Lasse Reichstein Nielsen, 2015-11-23 13:33:26]

lrn@google.com changed reviewers:
+ floitsch@google.com

[kevmoo, 2015-11-23 16:48:08]

kevmoo@google.com changed reviewers:
+ kevmoo@google.com

[kevmoo, 2015-11-23 16:48:08]

This is missing one critical thing...

[floitsch, 2015-11-23 19:11:01]

LGTM, once we agree on the name.

https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart
File sdk/lib/core/date_time.dart (right):

https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart...
sdk/lib/core/date_time.dart:383: DateTime replace({int year,
I think we discussed this already on another class, but I'm really not happy
with the name `replace`. It sounds way too much like an in-place operation.

Can we use `with`?

`date.with(year: 1000)` ?

[kevmoo, 2015-11-23 19:15:35]

On 2015/11/23 19:11:01, floitsch wrote:
> LGTM, once we agree on the name.
> 
> https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart
> File sdk/lib/core/date_time.dart (right):
> 
>
https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart...
> sdk/lib/core/date_time.dart:383: DateTime replace({int year,
> I think we discussed this already on another class, but I'm really not happy
> with the name `replace`. It sounds way too much like an in-place operation.
> 
> Can we use `with`?
> 
> `date.with(year: 1000)` ?

...although it might be nice to be consistent with what we did in Uri

[sra1, 2015-11-23 21:15:14]

sra@google.com changed reviewers:
+ sra@google.com

[sra1, 2015-11-23 21:15:14]

https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart
File sdk/lib/core/date_time.dart (right):

https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart...
sdk/lib/core/date_time.dart:372: * Equivalent to createing a new `DateTime`
using `new DateTime`
creating

https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart...
sdk/lib/core/date_time.dart:383: DateTime replace({int year,
On 2015/11/23 19:11:01, floitsch wrote:
> I think we discussed this already on another class, but I'm really not happy
> with the name `replace`. It sounds way too much like an in-place operation.
> 
> Can we use `with`?
> 
> `date.with(year: 1000)` ?

I would like to see `with` too.

We should have used `with` on Uri. We still can.
What do we want to do for the next 5 classes that have this kind of 'functional
update'?

[floitsch, 2015-11-23 22:13:49]

On 2015/11/23 21:15:14, sra1 wrote:
> https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart
> File sdk/lib/core/date_time.dart (right):
> 
>
https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart...
> sdk/lib/core/date_time.dart:372: * Equivalent to createing a new `DateTime`
> using `new DateTime`
> creating
> 
>
https://codereview.chromium.org/1472803003/diff/1/sdk/lib/core/date_time.dart...
> sdk/lib/core/date_time.dart:383: DateTime replace({int year,
> On 2015/11/23 19:11:01, floitsch wrote:
> > I think we discussed this already on another class, but I'm really not happy
> > with the name `replace`. It sounds way too much like an in-place operation.
> > 
> > Can we use `with`?
> > 
> > `date.with(year: 1000)` ?
> 
> I would like to see `with` too.
> 
> We should have used `with` on Uri. We still can.
Deprecate "replace" and add "with" ?

> What do we want to do for the next 5 classes that have this kind of
'functional
> update'?

Ah. I knew we had discussed this just recently...
Why didn't we take "with" on Uri? I'm pretty sure I mentioned it then.

[Lasse Reichstein Nielsen, 2015-11-24 06:48:33]

On 2015/11/23 at 22:13:49, floitsch wrote:
> 
> Ah. I knew we had discussed this just recently...
> Why didn't we take "with" on Uri? I'm pretty sure I mentioned it then.

Most likely because "with" is a reserved word in Dart - it's a JavaScript
keyword.
Also, I don't really like `with`. It conveys no meaning by itself.
I acknowledge that it reads somewhat well with named parameters, but you can't
guess what `foo.with()` does.
The `replace` name has precedent from String.

[kevmoo, 2015-11-24 19:18:16]

On 2015/11/23 16:48:08, kevmoo wrote:
> This is missing one critical thing...

With less obfuscation: please add an entry to the CHANGELOG

[Lasse Reichstein Nielsen, 2015-11-25 09:04:23]

On 2015/11/23 at 22:13:49, floitsch wrote:
> Ah. I knew we had discussed this just recently...

Indeed. The problem here is that we have immutable data, so "modifying
operations" instead return a new instance.
That means that we have a modifying operation, so any reasonable name for it
will sound like it is updating the object. That is a curse that any String has
lived with forever, and other classes likely too.
Also, "replace" isn't necessarily the only possible method that has the problem,
so even if we can find a clever name for that, it doesn't solve the general
problem.

I think using the classical "modifying" method names and returning a new
instance is the best way to handle such "modifiable" immutable data structures.
Coming up with new names for all the operations will be at least as confusing to
the user. E.g.,
  replace/update -> with (if possible)
  addAll         -> appendedWith
  removeAll      -> without
  retainAll      -> ?
  add            -> ?
  remove         -> ?

So, I maintain that "replace" is a good name for the operation of replacing some
part of the object with another on an immutable object.

[sra1, 2015-12-01 01:04:51]

Description was changed from

==========
Add DateTime.replace.

Allows replacing part of a DateTime without touching the rest.

Fixes issue #24644
BUG= https://github.com/dart-lang/sdk/issues/24644
==========

to

==========
Add DateTime.replace.

Allows replacing part of a DateTime without touching the rest.

Fixes issue #24644
BUG= https://github.com/dart-lang/sdk/issues/24644
==========

[sra1, 2015-12-01 01:04:52]

sra@google.com changed reviewers:
- sra@google.com

[Lasse Reichstein Nielsen, 2015-12-10 10:25:42]

So no better suggestion that "replace" has come up, so I say we use it.

Speak now, or forever hold your peace.

[floitsch, 2015-12-10 22:40:44]

On 2015/12/10 10:25:42, Lasse Reichstein Nielsen wrote:
> So no better suggestion that "replace" has come up, so I say we use it.
> 
> Speak now, or forever hold your peace.

I still don't like it... :(
This is one of the instances where Scheme's "!" for mutation would help...

Let's discuss this once more when I'm in Aarhus. I doubt we will find a better
solution, but waiting a few more days shouldn't be a big deal (I doubt there
will be conflicting commits).