[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 13:31:17 PDT 2015


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gemtalksystems.com/mailman/private/glass/attachments/20150810/09da72b9/attachment-0001.html>


More information about the Glass mailing list