[squeak-dev] Time>addSeconds: ignores nanos ivar value
tim at rowledge.org
Mon Jul 20 21:00:40 UTC 2020
It isn't my field at all but since time is not really quantised until Plank-time scale, shouldn't we be using floating point for seconds? Or is the argument that keeping a couple of integers is 'cheaper'?
Also, should the added seconds be restricted to an integer number of seconds? What ought happen if a float is passed in?
> On 2020-07-20, at 1:45 PM, David T. Lewis <lewis at mail.msen.com> wrote:
> On Mon, Jul 20, 2020 at 01:08:57PM -0700, tim Rowledge wrote:
>> Time >addSeconds: completely ignores the nanos ivar value and so
>> Time now -> (say) 12:51:47.418205 pm
>> send that #addSeconds: 2 -> 12:51:49 pm
>> Which is incorrect. Actually *fixing* it seems to be a bit more complex than just "add the nanoseconds stuff".
>> Should #asSeconds be truncating the seconds and ignoring the nanoseconds? That's also an issue for #addTime: and #subtractTime: (wait, no #subtractSeconds: ?)
>> Chronology-afficonados assemble!
> It looks broken to me. And deserving of a unit test to document the expected result.
> In Squeak 3.8 #addSeconds: seems to do the same thing, but "Time now" gives
> an instance with nanos equal to zero, so possibly it is old behavior that
> was just not noticed.
> In DateAndTime, we introduced #asExactSeconds to allow backward compatibility
> with the original #asSeconds that answered whole seconds. Maybe something
> similar would be needed for Time.
> Or maybe it should just be this:
> Time class>>addSeconds: nSeconds
> "Answer a Time that is nSeconds after the receiver."
> ^ self class
> seconds:seconds + nSeconds
> nanoSeconds: nanos
> Either way a unit test is the first step. The existing TimeTest>>testAddSeconds
> does not provide any coverage for instances of Time with non-zero nanos.
tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim
Press [ESC] to detonate or any other key to explode.
More information about the Squeak-dev