Smalltalk › Pharo › Pharo Smalltalk Developers

Issue 3806 in pharo: Duration>>hours is broken

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

21 messages Options

pharo

Issue 3806 in pharo: Duration>>hours is broken

Status: New
Owner: ----

New issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

Steps to reproduce:

1. self assert: 2 days hours = 48
2. do
3. it

it will fail

It's a bug because of what is said in the comment. But please don't even
think in fixing it by changing the comment. Even a toddler would expect to
see 48 hours in a duration of two days.

BTW: I've just saw this and I'm probably going to make a patch

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Comment #1 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

deeper than I though...

#minutes and #seconds are broken too

#asSeconds is doing the #seconds work but the thing should be the other way
around (#asSeconds should be synonym of the "#seconds" of the duration)

and if for any nerd reason you want the seconds of the last day of the
duration then, if the image is generous with that nerdicism, then you
should be specific and get it (so #lastDaySeonds should exists)

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Comment #2 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

here is the patch (use it at your own risk)

Attachments:
pharoDurationPatch-sebastian_sastre.1.mcz 1.9 KB

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Comment #3 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

Thanks.
Do you have some tests?

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Updates:
Status: Fixed
Labels: Milestone-1.3

Comment #4 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

(No comment was entered for this change.)

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Comment #5 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

While I can second Sebastians' complains based in intuitivity, I got
confused with the redaction of the ANSI standard, which I have only a 1.9
Draft in PDF form:

5.8.2.11 Message: hours
Synopsis
Answer the number of complete hours in the receiver.
Definition: <Duration>
Answer an <integer> between -23 and 23 inclusive that represents the number
of complete hours
in the receiver, after the number of complete days has been removed. If the
receiver is less than
<Duration factory> #zero then the result will be less than or equal to 0.
Return Values
<integer> unspecified
Errors
None

This text seems to support the implementation of the method which is
attempting to give the number of fractional part of the duration. It does
not seem to be a conversion protocol, but rather an equivalent to the mod
operation for hours, or seconds or whatever.

Test this:

(DateAndTime now - DateAndTime today) hours.

In my image, at the time I tested it gave a correct "16".

John Toohey

Re: Issue 3806 in pharo: Duration>>hours is broken

I just finished debugging a production server that had a similar issue. Basically I need to select from a collection all events that were opening in the next 24 hours. I have code like :-

hours := (each eventOpenTime - timeNow) hours.

hours between: 0 and: 24.

I thought this was working before, but debugging the server today, I saw that every event was between 0:23. The #hours message only returned the hours portion of a time, not the amount of hours. I had to change the code to :-

hours := (((each eventOpenTIme - timeNow) asSeconds) / 60 / 60) asInteger.

Anyhow, I guess the name was misleading to me, as there is no #asHours message, but these is #asSeconds, and an #hours and #minutes, but no #seconds.

On Fri, Mar 11, 2011 at 14:04, <[hidden email]> wrote:

Comment #5 on issue 3806 by [hidden email]: Duration>>hours is broken

http://code.google.com/p/pharo/issues/detail?id=3806

While I can second Sebastians' complains based in intuitivity, I got confused with the redaction of the ANSI standard, which I have only a 1.9 Draft in PDF form:

5.8.2.11 Message: hours
Synopsis
Answer the number of complete hours in the receiver.
Definition: <Duration>
Answer an <integer> between -23 and 23 inclusive that represents the number of complete hours
in the receiver, after the number of complete days has been removed. If the receiver is less than
<Duration factory> #zero then the result will be less than or equal to 0.
Return Values
<integer> unspecified
Errors
None

This text seems to support the implementation of the method which is attempting to give the number of fractional part of the duration. It does not seem to be a conversion protocol, but rather an equivalent to the mod operation for hours, or seconds or whatever.

Test this:

(DateAndTime now - DateAndTime today) hours.

In my image, at the time I tested it gave a correct "16".

--
~JT

Stéphane Ducasse

Re: Issue 3806 in pharo: Duration>>hours is broken

tx
Giving feedback on issue is a really important way to contribute
John could you copy and paste your mail in the issue?

Stef

On Mar 11, 2011, at 10:22 PM, John Toohey wrote:

> I just finished debugging a production server that had a similar issue. Basically I need to select from a collection all events that were opening in the next 24 hours. I have code like :-
>
> hours := (each eventOpenTime - timeNow) hours.
> hours between: 0 and: 24.
>
> I thought this was working before, but debugging the server today, I saw that every event was between 0:23. The #hours message only returned the hours portion of a time, not the amount of hours. I had to change the code to :-
>
> hours := (((each eventOpenTIme - timeNow) asSeconds) / 60 / 60) asInteger.
>
> Anyhow, I guess the name was misleading to me, as there is no #asHours message, but these is #asSeconds, and an #hours and #minutes, but no #seconds.
>
>
> On Fri, Mar 11, 2011 at 14:04, <[hidden email]> wrote:
>
> Comment #5 on issue 3806 by [hidden email]: Duration>>hours is broken
>
> http://code.google.com/p/pharo/issues/detail?id=3806
>
> While I can second Sebastians' complains based in intuitivity, I got confused with the redaction of the ANSI standard, which I have only a 1.9 Draft in PDF form:
>
> 5.8.2.11 Message: hours
> Synopsis
> Answer the number of complete hours in the receiver.
> Definition: <Duration>
> Answer an <integer> between -23 and 23 inclusive that represents the number of complete hours
> in the receiver, after the number of complete days has been removed. If the receiver is less than
> <Duration factory> #zero then the result will be less than or equal to 0.
> Return Values
> <integer> unspecified
> Errors
> None
>
> This text seems to support the implementation of the method which is attempting to give the number of fractional part of the duration. It does not seem to be a conversion protocol, but rather an equivalent to the mod operation for hours, or seconds or whatever.
>
> Test this:
>
> (DateAndTime now - DateAndTime today) hours.
>
> In my image, at the time I tested it gave a correct "16".
>
>
>
>
>
>
>
>
> --
> ~JT
>
>

John Toohey

Re: Issue 3806 in pharo: Duration>>hours is broken

Done.

On Sat, Mar 12, 2011 at 03:05, Stéphane Ducasse <[hidden email]> wrote:

tx
Giving feedback on issue is a really important way to contribute
John could you copy and paste your mail in the issue?

Stef

On Mar 11, 2011, at 10:22 PM, John Toohey wrote:

> I just finished debugging a production server that had a similar issue. Basically I need to select from a collection all events that were opening in the next 24 hours. I have code like :-
>
> hours := (each eventOpenTime - timeNow) hours.
> hours between: 0 and: 24.
>
> I thought this was working before, but debugging the server today, I saw that every event was between 0:23. The #hours message only returned the hours portion of a time, not the amount of hours. I had to change the code to :-
>
> hours := (((each eventOpenTIme - timeNow) asSeconds) / 60 / 60) asInteger.
>
> Anyhow, I guess the name was misleading to me, as there is no #asHours message, but these is #asSeconds, and an #hours and #minutes, but no #seconds.
>
>
> On Fri, Mar 11, 2011 at 14:04, <[hidden email]> wrote:
>
> Comment #5 on issue 3806 by [hidden email]: Duration>>hours is broken
>
> http://code.google.com/p/pharo/issues/detail?id=3806
>
> While I can second Sebastians' complains based in intuitivity, I got confused with the redaction of the ANSI standard, which I have only a 1.9 Draft in PDF form:
>
> 5.8.2.11 Message: hours
> Synopsis
> Answer the number of complete hours in the receiver.
> Definition: <Duration>
> Answer an <integer> between -23 and 23 inclusive that represents the number of complete hours
> in the receiver, after the number of complete days has been removed. If the receiver is less than
> <Duration factory> #zero then the result will be less than or equal to 0.
> Return Values
> <integer> unspecified
> Errors
> None
>
> This text seems to support the implementation of the method which is attempting to give the number of fractional part of the duration. It does not seem to be a conversion protocol, but rather an equivalent to the mod operation for hours, or seconds or whatever.
>
> Test this:
>
> (DateAndTime now - DateAndTime today) hours.
>
> In my image, at the time I tested it gave a correct "16".
>
>
>
>
>
>
>
>
> --
> ~JT
>
>

--
~JT

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

In reply to this post by pharo

Comment #6 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

I just finished debugging a production server that had a similar issue.
Basically I need to select from a collection all events that were opening
in the next 24 hours. I have code like :-

hours := (each eventOpenTime - timeNow) hours.
hours between: 0 and: 24.

I thought this was working before, but debugging the server today, I saw
that every event was between 0:23. The #hours message only returned the
hours portion of a time, not the amount of hours. I had to change the code
to :-

hours := (((each eventOpenTIme - timeNow) asSeconds) / 60 / 60) asInteger.

Anyhow, I guess the name was misleading to me, as there is no #asHours
message, but these is #asSeconds, and an #hours and #minutes, but no
#seconds.

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Comment #7 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

Just testing my understanding, then: given the wording of the ANSI
Standard, and the present behaviour the "Fixed" status is in fact "is not a
bug" and no changes will be commited to the code, right?

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Updates:
Status: Accepted

Comment #8 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

No really. What is the consensus?

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Updates:
Status: FixProposed

Comment #9 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

(No comment was entered for this change.)

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

Comment #10 on issue 3806 by [hidden email]: Duration>>hours is
broken
http://code.google.com/p/pharo/issues/detail?id=3806

Well I think we need to rephrase that to: What is the consensus on changing
these protocols which will break ANSI compliance.

Returning 48 hours for (2 days) hours, is obviously non conforming the ANSI
standard, except if from the draft I have access to it was materially
changed to mean something resembling the OP request.

I think the sixth comment brings a compromise solution, which is to
implement the #asHours (perhaps even #asMinutes and #asSeconds) message.
Leaving those ANSI protocol alone.

Brent Pinkney-2

Re: Issue 3806 in pharo: Duration>>hours is broken

In reply to this post by pharo

> Steps to reproduce:
>
> 1. self assert: 2 days hours = 48
> 2. do
> 3. it
>
> it will fail
>
> It's a bug because of what is said in the comment. But please don't even
> think in fixing it by changing the comment. Even a toddler would expect to
> see 48 hours in a duration of two days.
>

Hi, I am that mental toddler that coded up the ANSI compliant date and time
package.

Please refer to ANSI spec §5.8.2.11. I promise it will only take a minute.

Instead of a polite enquiry as to why the implementation is such, you had to
define obvious for yourself and chuck in an insult. Bona fide arsehole

PS. Try now guess why there is both #seconds and #asSeconds.

Brent

Geert Claes

Re: Issue 3806 in pharo: Duration>>hours is broken

Administrator

Brent Pinkney-2 wrote

Hi, I am that mental toddler that coded up the ANSI compliant date and time
package.

Please refer to ANSI spec §5.8.2.11. I promise it will only take a minute.

Instead of a polite enquiry as to why the implementation is such, you had to
define obvious for yourself and chuck in an insult. Bona fide arsehole

PS. Try now guess why there is both #seconds and #asSeconds.

Brent

Hi Brent, I think not everyone here knows the exact content from Smalltalk ANSI. When someone questions an implementation it is probably a good idea to first cross-reference the ANSI standard, so I have added a link to the ANSI INCITS 319-1998 (R2007) on world.st/about and also made the draft PDF available. I noticed you mentioned someone insulted you? I probably missed that bit and I think everyone here agrees that there is no need to insult anyone, we are all adults here ... well at least I think so :)

ANSI 5.8.2.11 Draft wrote

5.8.2.11 Message: hours
Synopsis
Answer the number of complete hours in the receiver.
Definition: <Duration>
Answer an <integer> between -23 and 23 inclusive that represents the number of complete hours
in the receiver, after the number of complete days has been removed. If the receiver is less than
<Duration factory> #zero then the result will be less than or equal to 0.
Return Values
<integer> unspecified
Errors
None

ps. I am not sure what the "(R2007)" means since the draft PDF looks like it is still from 1997. Does anyone know if there is a way to make this information available for free rather than 30 USD from the ansi.org website?

pharo

Re: Issue 3806 in pharo: Duration>>hours is broken

In reply to this post by pharo

Comment #11 on issue 3806 by [hidden email]: Duration>>hours is broken
http://code.google.com/p/pharo/issues/detail?id=3806

My 2c:
I agree with cesar:
- Keep ANSI compliancy for #hours.
- Add #asHours, doing what OP expected.
- Explain the difference in comments, so noone thinks the implementer
didn't know what he was doing :)

#asSeconds is already ANSI, though the draft of the standard I have states
it should include fractional parts.
I suppose nanos instvar was added after the method was written, either way
it should be extended to reflect the current situation.
I suggest implementing #asHours and #asMinutes including fractions too, to
keep it consistent.

Brent Pinkney-2

Re: Issue 3806 in pharo: Duration>>hours is broken

In reply to this post by Geert Claes

Hi,

Lets end this thread quickly - I don't think it warrants a flame war.

I took a hard line on the "toddler" bit as there is a tendency for some, often new
members in the community to jump to conclusions about what is a pretty elegent
platform (Squeak), and presume oversight or errors by default.
Hence the "bona fide..." bit :)

Further, arguments starting with "...everyone knows X..." is just not a reasonable
starting point, post Enlightenment and outside theocracies.

Many things about date and time libraries are just not obvious, which is why by
Java 1.2 they already had 6 different date/time classes (if I recall).

A better approach is to enquire as to why something is the way it is. By
way of example, the loose consensus at the time was to follow the draft
ANSI spec only, rather than pay ANSI to "get our spec back".

Hell, my initials are all over the code, just ask.

Cheers

Brent the Tolerant and Approachable

Stéphane Ducasse

Re: Issue 3806 in pharo: Duration>>hours is broken

I like your signature :)

On Mar 14, 2011, at 4:22 PM, Brent Pinkney wrote:

> Hi,
>
> Lets end this thread quickly - I don't think it warrants a flame war.
>
> I took a hard line on the "toddler" bit as there is a tendency for some, often new
> members in the community to jump to conclusions about what is a pretty elegent
> platform (Squeak), and presume oversight or errors by default.
> Hence the "bona fide..." bit :)
>
> Further, arguments starting with "...everyone knows X..." is just not a reasonable
> starting point, post Enlightenment and outside theocracies.
>
> Many things about date and time libraries are just not obvious, which is why by
> Java 1.2 they already had 6 different date/time classes (if I recall).
>
> A better approach is to enquire as to why something is the way it is. By
> way of example, the loose consensus at the time was to follow the draft
> ANSI spec only, rather than pay ANSI to "get our spec back".
>
> Hell, my initials are all over the code, just ask.
>
> Cheers
>
> Brent the Tolerant and Approachable
>

Geert Claes

Re: Issue 3806 in pharo: Duration>>hours is broken

Administrator

In reply to this post by Brent Pinkney-2

Brent Pinkney-2 wrote

...
Hell, my initials are all over the code, just ask.

Cheers

Brent the Tolerant and Approachable

No worries, I fully agree with the draft ANSI spec approach and that everyone should be able to just ask

Geert the diplomat and peace-keeper :)