3 ms·
The problem here isn't with time.Time, it's with reflect.DeepEqual. Using it in your code to compare arbitrary data is almost always incorrect. And IMO, adding
by barsonme 5y ago
The problem here isn't with time.Time, it's with reflect.DeepEqual. Using it in your code to compare arbitrary data is almost always incorrect. And IMO, adding it to the stdlib was a mistake.
time.Time values should only be compared for equality using the Equal method (or IsZero), but reflect.DeepEqual does not know that.
This can happen with any type, not just time.Time. For example, I have a decimal library. It can represent 200 as either (mantissa=2, exponent=2) or (mantissa=200, exponent=0). Comparing those two numbers for equality will return true, but comparing them with reflect.DeepEqual will return false.
So, it's not a "nasty bit of undefined timezone behavior." It's somebody misusing an API.
- tialaramex 5y agoThis feels to me like maybe a good example for operator overloading. Often the "good" case of operator overloading is given as some fancy numeric type, like Complex or Matrix or something, for which some arithmetic makes sense but the fundamental integer and floating point operators don't cut it. But here is a case where programmer defined overloading of equality shows a benefit, if each type (including time.Time) takes responsibility for knowing how to compare that type then nobody else needs to know this detail and the natural comparison operators work, rather than needing to awkwardly define == and Equal separately and then forever live with people choosing the wrong one by mistake. After all, in the absence of such an overload (and I agree having it would interfere with Go's goal of simplicity) this would likely happen even without reflect.DeepEqual because people want to do this, and "You can't do this safely" is unlikely to have better results in Go than other languages.
- formerly_proven 5y agoThis excerpt from the Go docs is an excellent case for (some) operating overloading: > Note that the Go == operator compares not just the time instant but also the Location and the monotonic clock reading. Therefore, Time values should not be used as map or database keys without first guaranteeing that the identical Location has been set for all values, which can be achieved through use of the UTC or Local method, and that the monotonic clock reading has been stripped by setting t = t.Round(0). In general, prefer t.Equal(u) to t == u, since t.Equal uses the most accurate comparison available and correctly handles the case when only one of its arguments has a monotonic clock reading.
- tialaramex 5y agoFurther thought: It's probably also necessary to spell out what exactly "equality" means for your operator so that programmers know what they're supposed to be implementing, and what they're getting from other programmers. It's interesting for example that Rust gets away with assert_eq!(vec![1,2,5,9],[1,2,5,9]); That's claiming a vector and an array are equal since they have the same integers in them. Neither of Go's notions of equality think these are equal but sure enough they do feel pretty equal in Rust, and in lots of places these would be interchangeable.
- wnoise 5y agoThe misusing the API did not cause anything. What it did is surface some unexpected behavior: "But calling Format on a time object, for some format strings (such as time.RFC3339), needs to load this information, so will do so implicitly as a side effect. After such a call to time.Format, all time.Time structs with the time.Local location will include this info, when they didn't before. This is the kind of surprising, spooky action-at-a-distance nonsense that well-designed libraries tend to avoid. Or they at least try!" That's the actual complaint, and that behavior remains whether or not anyone calls DeepEqual.