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

Mariano Martinez Peck via Glass glass at lists.gemtalksystems.com
Mon Aug 10 15:00:35 PDT 2015 

On Mon, Aug 10, 2015 at 6:16 PM, Dale Henrichs <
dale.henrichs at gemtalksystems.com> wrote:

> After submitting the bug, one of the engineers made a comment to the
> effect "the fix is not as simple as...", because of some incorrect usages,
> but I didn't try to track them down at the time ... but I thought that the
> problems were related to Date class>>newDay:year: (which does depend upon
> the "bad behavior" ...
>

Yes, newDay:year:  is one of the methods that need to call the bad
behavior.


>
> I looked a senders of Date class>>year:month:day: and I didn't see
> anything obviously wrong, but there are a number of senders of
> #newDay:year: and this guy _should_ be calling the #_new:... method to get
> the right answers for things like:
>
>   Date newDay: 219 monthNumber: 1 year: 2015
>
> but then this guy goes through a caller of Date class>>newDay:year: so it
> should be fixable ...
>
>
Yes.


> Do you have an example that does not go through Date class>>newDay:year:?
>
>
Yes, but it's not easy to statically analyze which could be problematic.

Date >> addMonths:    does also break it:


(Date fromString: '2015-05-31') addMonths: 1

Tries to generate 31 of June...

Same for #addYears: but more complicated to reproduce:

(Date fromString: '2012-02-29') addYears: 1

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?

Do you have found another wrong sender?

Thanks!


> Dale
>
>
> On 08/10/2015 01:31 PM, Mariano Martinez Peck wrote:
>
> 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 <
> marianopeck at gmail.com> 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 <
>> dale.henrichs at gemtalksystems.com> 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 <
>>> marianopeck at gmail.com> wrote:
>>>
>>>>
>>>> On Mon, Aug 10, 2015 at 1:50 PM, Dale Henrichs <
>>>> dale.henrichs at gemtalksystems.com> 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 <
>>>>> dale.henrichs at gemtalksystems.com> 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 <
>>>>>> glass at lists.gemtalksystems.com> 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 listGlass at lists.gemtalksystems.comhttp://lists.gemtalksystems.com/mailman/listinfo/glass
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Glass mailing list
>>>>>>> Glass at lists.gemtalksystems.com
>>>>>>> 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
>
>
>


-- 
Mariano
http://marianopeck.wordpress.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gemtalksystems.com/mailman/private/glass/attachments/20150810/598f0b98/attachment-0001.html>

More information about the Glass mailing list