Hacker News Reader

Comment by tantalor | Hacker News Reader

What does mutability have to do with this bug?

The old code was

    targetEndDate = new Date();
    targetEndDate.setFullYear(endDate.getFullYear());
    targetEndDate.setMonth(endDate.getMonth());
    targetEndDate.setDate(endDate.getDate());

Even if the endDate is valid, this can fail, because today is the 31st, which doesn't exist in endDate's month.

If you just did:

    targetEndDate = createDate(endDate.getFullYear(), endDate.getMonth(), endDate.getDate()); 
    // createDate is an intentional placeholder, I didn't want to double-check the actual syntax

you wouldn't pass through the transitional stage where you have the new month but today's day of the month, which is what causes the bug.

In this case, using mutation on the individual fields makes you have to transition through an invalid state to get back to a valid one, and the JS date object does something unexpected (though I think there's nothing good to do here, an exception is probably the best you could do). So mutation really is at issue here. In general, mutation creates room for these kind of counter-intuitive state transitions to arise.

I agree, reading the example code was painful as someone who tries to write all code in a functional style.

> mutation on the individual fields makes you have to transition through an invalid state

If you want to do this mutation-style for some reason, you should use the builder pattern (or whatever it's called). So in this case you'd instantiate a DateBuilder instance instead of a Date instance, and finally call .date() to get the actual Date instance from the builder instance.

Better yet, in this particular case, don't use the builder pattern, and instead just provide a sensible set of constructors with appropriate defaults for unspecified arguments (e.g., 00:00 for unspecified time).

For a simple date/time class, the builder pattern reeks of pointless overengineering.

Oh certainly, didn't mean to imply it's great for a freaking date.

But I've seen the mutating pattern in other cases causing the weird state bug which could have avoided using a builder.

The direct "immutable" equivalent of the bug code (purely based on my experience with .NET Core immutable collections) would have the setX methods return a new immutable datetime with the field set as requested. So just "use an immutable type" wouldn't fix this bug.

"Change the code completely so you supply all 3 components at the same time" obviously fixes the bug, but that doesn't require an immutable type.

Yep, that's what I was getting at. Immutability is not a "silver bullet"

The .NET immutable collections seem to mostly avert problems like this. For example, ImmutableDictionary.Add<TKey, TValue>(key, value) throws an exception if key already exists with a different value (as determined by an explicit or implicit IEqualityComparer<TValue>).

.NET's (immutable) DateTime, on the other hand, has AddMonths(Int32), which sets the day to Min(original day number, last day of result month), which, while arguably reasonable, isn't obvious without reading the documentation.

My favorite counterintutive DateTime "mutator," however is AddMilliseconds(Double): prior to .NET 7, its floating-point argument is rounded to the nearest integer (!?!).

Reply on news.ycombinator.com