Comment of #newDay:monthNumber:year: is wrong (and I do not like the behavior either!)

Hi guys,

Just for the record, the comment of:

Date class >> newDay: day monthNumber: month year: year

"Creates and returns an instance of the receiver from the specified values.

 Generates an error if any of the values are out of range."

Is wrong since it does not throws and error and instead shifts the day. Example:

Date newDay: 31 monthNumber: 6 year: 2015   -> 07/01/2015   (June has only 30 days)

BTW... This is a frustrating behavior for me. Pharo would correctly throw a DateError, but GemStone shifts and gives me back another date. This could lead to problems quite hard to debug.  

Cheers, 

--

Mariano
http://marianopeck.wordpress.com

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

Thanks Dale. If a back port or workaround is easy for 3.1.0.7 I would appreciate that too.... I don't expext an official release... but maybe a workaround. I do not even care if I have to put it as an override as part of my packages... Of course...I can write myself the validation of ranges too...

But seems to me it sounds like a big bug... all users of such a method could have been getting a shifted date if they did not send a correct one...

On Fri, Aug 7, 2015 at 5:10 PM, Dale Henrichs via Glass <[hidden email]> wrote:

Mariano,

Thanks for the report, I've submitted an internal bug (45525) on this against 3.2.7 and 3.3.

Dale

On 08/07/2015 06:35 AM, Mariano Martinez Peck via Glass wrote:

Hi guys,

Just for the record, the comment of:

Date class >> newDay: day monthNumber: month year: year

"Creates and returns an instance of the receiver from the specified values.

 Generates an error if any of the values are out of range."

Is wrong since it does not throws and error and instead shifts the day. Example:

Date newDay: 31 monthNumber: 6 year: 2015   -> 07/01/2015   (June has only 30 days)

BTW... This is a frustrating behavior for me. Pharo would correctly throw a DateError, but GemStone shifts and gives me back another date. This could lead to problems quite hard to debug.  

Cheers, 

--

Mariano
http://marianopeck.wordpress.com

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

--

Mariano
http://marianopeck.wordpress.com

Hi Dale and others,

The Smalltalk fix is not possible to put in Date class #newDay:monthNumber:year:  (which is the ideal place) because since the primitive does NOT fail, then the smalltalk code below is never executed. Therefore, the fix must be at the sender of this guy. Unfortunately, there are a few :(

The "fix" I propose is the following. Please let me know if this looks correct:

New helper method:

Date class >> numberOfDaysIn: month year: aYear

((month == 1) or: [(month == 3) or: [(month == 5) or: [(month == 7) or:

   [(month == 8) or: [(month == 10) or: [(month == 12)]]]]]])

   ifTrue: [^ 31].

((month == 4) or: [(month == 6) or: [(month == 9) or: [(month == 11)]]])

   ifTrue: [^ 30].

(((aYear \\ 100) == 0)

   ifTrue: [ ((aYear \\ 400) == 0)]

   ifFalse: [ ((aYear \\ 4) == 0) ])

  ifTrue: [^ 29].

^ 28

And then, change each sender of #newDay:monthNumber:year:  to add the 2 lines below:

Date class >> year: year month: month day: day

(month between: 1 and: 12) ifFalse: [ self error: 'Incorrect specified month: ', month asString].

(day between: 1 and: (self numberOfDaysIn: month year: year)) ifFalse: [ self error: 'Incorrect specified day: ', day asString]. 

^self newDay: day monthNumber: month year: year

Does it look correct to you?

Best, 

 

On Fri, Aug 7, 2015 at 5:24 PM, Dale Henrichs <[hidden email]> wrote:

Mariano,

Since the creation of a date is a primitive call, the official fix will be in C code (I think) ...  it looks like there is no validation for the day of the month, so a straight forward Smalltalk patch should be possible ...

Dale

On 08/07/2015 01:18 PM, Mariano Martinez Peck wrote:

Thanks Dale. If a back port or workaround is easy for 3.1.0.7 I would appreciate that too.... I don't expext an official release... but maybe a workaround. I do not even care if I have to put it as an override as part of my packages... Of course...I can write myself the validation of ranges too...

But seems to me it sounds like a big bug... all users of such a method could have been getting a shifted date if they did not send a correct one...

On Fri, Aug 7, 2015 at 5:10 PM, Dale Henrichs via Glass <[hidden email]> wrote:

Mariano,

Thanks for the report, I've submitted an internal bug (45525) on this against 3.2.7 and 3.3.

Dale

On 08/07/2015 06:35 AM, Mariano Martinez Peck via Glass wrote:

Hi guys,

Just for the record, the comment of:

Date class >> newDay: day monthNumber: month year: year

"Creates and returns an instance of the receiver from the specified values.

 Generates an error if any of the values are out of range."

Is wrong since it does not throws and error and instead shifts the day. Example:

Date newDay: 31 monthNumber: 6 year: 2015   -> 07/01/2015   (June has only 30 days)

BTW... This is a frustrating behavior for me. Pharo would correctly throw a DateError, but GemStone shifts and gives me back another date. This could lead to problems quite hard to debug.  

Cheers, 

--

Mariano
http://marianopeck.wordpress.com

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

OK....I share the Topaz script with the fix (with the improved suggestion of Dale) in case others are in the same situation. 

The categories of the created methods looks like wrong... (i attach a screenshot) they look like a string in the browser. So...likely I am setting the category wrong.

If someone knows how to do it properly, let met know. Remember that since I am using SysterUser I cannot use any of the methods defined in packages from Monticello, or Squeak, etc. They must be in the core.

Thanks,

set user SystemUser pass XXX

run 

Date  class compileMethod: ' numberOfDaysIn: month year: aYear

((month == 1) or: [(month == 3) or: [(month == 5) or: [(month == 7) or:

   [(month == 8) or: [(month == 10) or: [(month == 12)]]]]]])

   ifTrue: [^ 31].

((month == 4) or: [(month == 6) or: [(month == 9) or: [(month == 11)]]])

   ifTrue: [^ 30].

(((aYear \\ 100) == 0)

   ifTrue: [ ((aYear \\ 400) == 0)]

   ifFalse: [ ((aYear \\ 4) == 0) ])

  ifTrue: [^ 29].

^ 28

'

dictionaries: GsSession currentSession symbolList

category: '''Accessing'''

environmentId: 0.

Date  class compileMethod: '_newDay: day monthNumber: month year: year

<primitive: 316>

day _validateClass: SmallInteger .

month _validateClass: SmallInteger .

year _validateClass: SmallInteger .

^ self _primitiveFailed: #newDay:monthNumber:year:

       args: { day . month . year }

'

dictionaries: GsSession currentSession symbolList

category: '''Instance Creation'''

environmentId: 0.

Date  class compileMethod: 'newDay: day monthNumber: month year: year

(month between: 1 and: 12) ifFalse: [ self error: ''Incorrect specified month: '', month asString].

(day between: 1 and: (self numberOfDaysIn: month year: year)) ifFalse: [ self error: ''Incorrect specified day: '', day asString]. 

^ self _newDay: day monthNumber: month year: year

'

dictionaries: GsSession currentSession symbolList

category: '''Instance Creation'''

environmentId: 0.

System commit.

%

On Mon, Aug 10, 2015 at 2:38 PM, Mariano Martinez Peck <[hidden email]> wrote:

On Mon, Aug 10, 2015 at 1:50 PM, Dale Henrichs <[hidden email]> wrote:

Mariano,

An alternative is to create a method #_newDay:monthNumber:year: (as System user) that is a copy of the original primitive based method ,,, and then implement #newDay:monthNumber:year: to call the prim method after testing values  ...

One caution here is that you should change Date class>>newDay:year: to call the #_newDay:... method ...

Indeed, that's another solution. And yes, I also found out it needs a special compile primitive  user permission (that's why you said to use System user).

mmmm probably yours is better. 

Thanks for the idea.

 

Dale

On 08/10/2015 06:45 AM, Mariano Martinez Peck wrote:

Hi Dale and others,

The Smalltalk fix is not possible to put in Date class #newDay:monthNumber:year:  (which is the ideal place) because since the primitive does NOT fail, then the smalltalk code below is never executed. Therefore, the fix must be at the sender of this guy. Unfortunately, there are a few :(

The "fix" I propose is the following. Please let me know if this looks correct:

New helper method:

Date class >> numberOfDaysIn: month year: aYear

((month == 1) or: [(month == 3) or: [(month == 5) or: [(month == 7) or:

   [(month == 8) or: [(month == 10) or: [(month == 12)]]]]]])

   ifTrue: [^ 31].

((month == 4) or: [(month == 6) or: [(month == 9) or: [(month == 11)]]])

   ifTrue: [^ 30].

(((aYear \\ 100) == 0)

   ifTrue: [ ((aYear \\ 400) == 0)]

   ifFalse: [ ((aYear \\ 4) == 0) ])

  ifTrue: [^ 29].

^ 28

And then, change each sender of #newDay:monthNumber:year:  to add the 2 lines below:

Date class >> year: year month: month day: day

(month between: 1 and: 12) ifFalse: [ self error: 'Incorrect specified month: ', month asString].

(day between: 1 and: (self numberOfDaysIn: month year: year)) ifFalse: [ self error: 'Incorrect specified day: ', day asString]. 

^self newDay: day monthNumber: month year: year

Does it look correct to you?

Best, 

 

On Fri, Aug 7, 2015 at 5:24 PM, Dale Henrichs <[hidden email]> wrote:

Mariano,

Since the creation of a date is a primitive call, the official fix will be in C code (I think) ...  it looks like there is no validation for the day of the month, so a straight forward Smalltalk patch should be possible ...

Dale

On 08/07/2015 01:18 PM, Mariano Martinez Peck wrote:

Thanks Dale. If a back port or workaround is easy for 3.1.0.7 I would appreciate that too.... I don't expext an official release... but maybe a workaround. I do not even care if I have to put it as an override as part of my packages... Of course...I can write myself the validation of ranges too...

But seems to me it sounds like a big bug... all users of such a method could have been getting a shifted date if they did not send a correct one...

On Fri, Aug 7, 2015 at 5:10 PM, Dale Henrichs via Glass <[hidden email]> wrote:

Mariano,

Thanks for the report, I've submitted an internal bug (45525) on this against 3.2.7 and 3.3.

Dale

On 08/07/2015 06:35 AM, Mariano Martinez Peck via Glass wrote:

Hi guys,

Just for the record, the comment of:

Date class >> newDay: day monthNumber: month year: year

"Creates and returns an instance of the receiver from the specified values.

 Generates an error if any of the values are out of range."

Is wrong since it does not throws and error and instead shifts the day. Example:

Date newDay: 31 monthNumber: 6 year: 2015   -> 07/01/2015   (June has only 30 days)

BTW... This is a frustrating behavior for me. Pharo would correctly throw a DateError, but GemStone shifts and gives me back another date. This could lead to problems quite hard to debug.  

Cheers, 

--

Mariano
http://marianopeck.wordpress.com

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

Ufff  after all this effort, I think we are wrong...

It seems #newDay:monthNumber:year: is indeed expected to receive "wrong" values. In fact, there are operations that rely on that, like #substractDays:

With my changes just try this:

Date today subtractDays: 2

And that will throw an error because that tries to do:

 Date newDay: 219 monthNumber: 1 year: 2015.

....

So... I think that then:

1) The comment of #newDay:monthNumber:year: is wrong and that method does NOT throw error if values are wrong.

2) THe methods to be fixed are SOME senders of #newDay:monthNumber:year:. The problem is to know which of the senders should be fixed and which do not because they depend on that weird shifting behavior when values are wrong. For sure #year:month:day: is one of the ones to be fixed. Can you help me figure out which other? Probably Date class >> julianDayNumber:   ,  Date class >> fromStream:usingFormat:   ,  

Honestly, I don't like the behavior of newDay... etc. The proper solution in my opinion would be to fix the weird senders that depend on that functionality. But..who knows who senders are those...and backward compatibility and ...

Thoughts?

On Mon, Aug 10, 2015 at 3:57 PM, Mariano Martinez Peck <[hidden email]> wrote:

Upps...you were right... sorry...I got the "I am in the string of the method source syndrom" hahaha

OK...this is done.

Thanks,

On Mon, Aug 10, 2015 at 3:48 PM, Dale Henrichs <[hidden email]> wrote:

Great!

The arg to category: is a string and it looks like you are using the printString of a String ...

  category: '''Accessing'''

should be:
 
  category: 'Accessing'

Otherwise looks good...

Dale

On 08/10/2015 11:28 AM, Mariano Martinez Peck wrote:

OK....I share the Topaz script with the fix (with the improved suggestion of Dale) in case others are in the same situation. 

The categories of the created methods looks like wrong... (i attach a screenshot) they look like a string in the browser. So...likely I am setting the category wrong.

If someone knows how to do it properly, let met know. Remember that since I am using SysterUser I cannot use any of the methods defined in packages from Monticello, or Squeak, etc. They must be in the core.

Thanks,

set user SystemUser pass XXX

run 

Date  class compileMethod: ' numberOfDaysIn: month year: aYear

((month == 1) or: [(month == 3) or: [(month == 5) or: [(month == 7) or:

   [(month == 8) or: [(month == 10) or: [(month == 12)]]]]]])

   ifTrue: [^ 31].

((month == 4) or: [(month == 6) or: [(month == 9) or: [(month == 11)]]])

   ifTrue: [^ 30].

(((aYear \\ 100) == 0)

   ifTrue: [ ((aYear \\ 400) == 0)]

   ifFalse: [ ((aYear \\ 4) == 0) ])

  ifTrue: [^ 29].

^ 28

'

dictionaries: GsSession currentSession symbolList

category: '''Accessing'''

environmentId: 0.

Date  class compileMethod: '_newDay: day monthNumber: month year: year

<primitive: 316>

day _validateClass: SmallInteger .

month _validateClass: SmallInteger .

year _validateClass: SmallInteger .

^ self _primitiveFailed: #newDay:monthNumber:year:

       args: { day . month . year }

'

dictionaries: GsSession currentSession symbolList

category: '''Instance Creation'''

environmentId: 0.

Date  class compileMethod: 'newDay: day monthNumber: month year: year

(month between: 1 and: 12) ifFalse: [ self error: ''Incorrect specified month: '', month asString].

(day between: 1 and: (self numberOfDaysIn: month year: year)) ifFalse: [ self error: ''Incorrect specified day: '', day asString]. 

^ self _newDay: day monthNumber: month year: year

'

dictionaries: GsSession currentSession symbolList

category: '''Instance Creation'''

environmentId: 0.

System commit.

%

On Mon, Aug 10, 2015 at 2:38 PM, Mariano Martinez Peck <[hidden email]> wrote:

On Mon, Aug 10, 2015 at 1:50 PM, Dale Henrichs <[hidden email]> wrote:

Mariano,

An alternative is to create a method #_newDay:monthNumber:year: (as System user) that is a copy of the original primitive based method ,,, and then implement #newDay:monthNumber:year: to call the prim method after testing values  ...

One caution here is that you should change Date class>>newDay:year: to call the #_newDay:... method ...

Indeed, that's another solution. And yes, I also found out it needs a special compile primitive  user permission (that's why you said to use System user).

mmmm probably yours is better. 

Thanks for the idea.

 

Dale

On 08/10/2015 06:45 AM, Mariano Martinez Peck wrote:

Hi Dale and others,

The Smalltalk fix is not possible to put in Date class #newDay:monthNumber:year:  (which is the ideal place) because since the primitive does NOT fail, then the smalltalk code below is never executed. Therefore, the fix must be at the sender of this guy. Unfortunately, there are a few :(

The "fix" I propose is the following. Please let me know if this looks correct:

New helper method:

Date class >> numberOfDaysIn: month year: aYear

((month == 1) or: [(month == 3) or: [(month == 5) or: [(month == 7) or:

   [(month == 8) or: [(month == 10) or: [(month == 12)]]]]]])

   ifTrue: [^ 31].

((month == 4) or: [(month == 6) or: [(month == 9) or: [(month == 11)]]])

   ifTrue: [^ 30].

(((aYear \\ 100) == 0)

   ifTrue: [ ((aYear \\ 400) == 0)]

   ifFalse: [ ((aYear \\ 4) == 0) ])

  ifTrue: [^ 29].

^ 28

And then, change each sender of #newDay:monthNumber:year:  to add the 2 lines below:

Date class >> year: year month: month day: day

(month between: 1 and: 12) ifFalse: [ self error: 'Incorrect specified month: ', month asString].

(day between: 1 and: (self numberOfDaysIn: month year: year)) ifFalse: [ self error: 'Incorrect specified day: ', day asString]. 

^self newDay: day monthNumber: month year: year

Does it look correct to you?

Best, 

 

On Fri, Aug 7, 2015 at 5:24 PM, Dale Henrichs <[hidden email]> wrote:

Mariano,

Since the creation of a date is a primitive call, the official fix will be in C code (I think) ...  it looks like there is no validation for the day of the month, so a straight forward Smalltalk patch should be possible ...

Dale

On 08/07/2015 01:18 PM, Mariano Martinez Peck wrote:

Thanks Dale. If a back port or workaround is easy for 3.1.0.7 I would appreciate that too.... I don't expext an official release... but maybe a workaround. I do not even care if I have to put it as an override as part of my packages... Of course...I can write myself the validation of ranges too...

But seems to me it sounds like a big bug... all users of such a method could have been getting a shifted date if they did not send a correct one...

On Fri, Aug 7, 2015 at 5:10 PM, Dale Henrichs via Glass <[hidden email]> wrote:

Mariano,

Thanks for the report, I've submitted an internal bug (45525) on this against 3.2.7 and 3.3.

Dale

On 08/07/2015 06:35 AM, Mariano Martinez Peck via Glass wrote:

Hi guys,

Just for the record, the comment of:

Date class >> newDay: day monthNumber: month year: year

"Creates and returns an instance of the receiver from the specified values.

 Generates an error if any of the values are out of range."

Is wrong since it does not throws and error and instead shifts the day. Example:

Date newDay: 31 monthNumber: 6 year: 2015   -> 07/01/2015   (June has only 30 days)

BTW... This is a frustrating behavior for me. Pharo would correctly throw a DateError, but GemStone shifts and gives me back another date. This could lead to problems quite hard to debug.  

Cheers, 

--

Mariano
http://marianopeck.wordpress.com

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

But as said, it's not easy. We must go one by one the senders of  #newDay:monthNumber:year:  and see if there are more problematics. So far:

Date class >> newDay: day year: year

Date >> addMonths:

Date >> addYears:

So for those 3 methods above, we should actually send the new message #_newDay:monthNumber:year: (wrong behavior) and let the rest use the fixed #newDay:monthNumber:year:

Thoughts?

Date  class compileMethod: ' numberOfDaysIn: month year: aYear

((month == 1) or: [(month == 3) or: [(month == 5) or: [(month == 7) or:

   [(month == 8) or: [(month == 10) or: [(month == 12)]]]]]])

   ifTrue: [^ 31].

((month == 4) or: [(month == 6) or: [(month == 9) or: [(month == 11)]]])

   ifTrue: [^ 30].

(((aYear \\ 100) == 0)

   ifTrue: [ ((aYear \\ 400) == 0)]

   ifFalse: [ ((aYear \\ 4) == 0) ])

  ifTrue: [^ 29].

^ 28

'

dictionaries: GsSession currentSession symbolList

category: 'Accessing'

environmentId: 0.

Date  class compileMethod: '_newDay: day monthNumber: month year: year

<primitive: 316>

day _validateClass: SmallInteger .

month _validateClass: SmallInteger .

year _validateClass: SmallInteger .

^ self _primitiveFailed: #newDay:monthNumber:year:

       args: { day . month . year }

'

dictionaries: GsSession currentSession symbolList

category: 'Instance Creation'

environmentId: 0.

Date  class compileMethod: 'newDay: day monthNumber: month year: year

(month between: 1 and: 12) ifFalse: [ self error: ''Incorrect specified month: '', month asString].

(day between: 1 and: (self numberOfDaysIn: month year: year)) ifFalse: [ self error: ''Incorrect specified day: '', day asString].

^ self _newDay: day monthNumber: month year: year

'

dictionaries: GsSession currentSession symbolList

category: 'Instance Creation'

environmentId: 0.

Date compileMethod: 'addMonths: anInteger

"Returns a Date that describes a date anInteger months later than that of the 

 receiver.

 This method attempts to keep the day of the month the same.  If the

 new month has fewer days than the receiver''s original month, then it

 truncates to the last day of the new month."

| yr month day newYear newMonth newDay newDate generatedDay |

yr := self year.

month := self month.

day := self day.

newMonth := month + anInteger.

newYear := yr + ((newMonth - 1) // 12).

newMonth := (newMonth - 1) \\ 12 + 1.

newDate := self class _newDay: day monthNumber: newMonth year: newYear.

generatedDay := newDate day.

(generatedDay ~= day)

  ifTrue: [

    newDay := newDate _daysInMonth: newMonth.

    newDate := self class _newDay: newDay monthNumber: newMonth year: newYear

    ].

^ newDate.

'

dictionaries: GsSession currentSession symbolList

category: 'Arithmetic'

environmentId: 0.

Date compileMethod: 'addYears: anInteger

"Returns a Date that describes a date anInteger years later than that of the 

 receiver."

| yr month day newYear newDay newDate generatedDay |

yr := self year.

month := self month.

day := self day.

newYear := yr + anInteger.

newDate := self class _newDay: day monthNumber: month year: newYear.

generatedDay := newDate day.

(generatedDay ~= day)

  ifTrue: [

    newDay := newDate _daysInMonth: month.

    newDate := self class _newDay: newDay monthNumber: month year: newYear

    ].

^ newDate.

'

dictionaries: GsSession currentSession symbolList

category: 'Arithmetic'

environmentId: 0.

Date class compileMethod: 'newDay: day year: year

"Creates and returns an instance of the receiver from the specified values.

 Generates an error if any of the values are out of range."

^ self _newDay: day monthNumber: 1 year: year.

'

dictionaries: GsSession currentSession symbolList

category: 'Instance Creation'

environmentId: 0.

System commit.

Double each \. So, \\\\ gets you \\ after escape processing.