[Glass] Comment of #newDay:monthNumber:year: is wrong (and I do not like the behavior either!)
Dale Henrichs via Glass
glass at lists.gemtalksystems.com
Mon Aug 10 14:16:23 PDT 2015
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" ...
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 ...
Do you have an example that does not go through Date class>>newDay:year:?
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 <mailto: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
> <mailto: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 <mailto:marianopeck at gmail.com>> wrote:
>>
>>
>> On Mon, Aug 10, 2015 at 1:50 PM, Dale Henrichs
>> <dale.henrichs at gemtalksystems.com
>> <mailto: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
>>> <mailto: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
>>>> <mailto: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 list
>>>>> Glass at lists.gemtalksystems.com <mailto:Glass at lists.gemtalksystems.com>
>>>>> http://lists.gemtalksystems.com/mailman/listinfo/glass
>>>>
>>>>
>>>> _______________________________________________
>>>> Glass mailing list
>>>> Glass at lists.gemtalksystems.com
>>>> <mailto: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/ff432043/attachment-0001.html>
More information about the Glass
mailing list