magic numbers in gtInspectorVariableValuePairs

Just curious what the magic numbers here relate to...

and can they be factored out to a meaningful method name?

Context>>gtInspectorVariableValuePairs

"This is a helper method that returns a collection of 

variable_name -> value

for the current object.

Subclasses can override it to specialize what appears in the variables presentation"

| bindings |

bindings := OrderedCollection new.

1 to: (self basicSize min: 21) do: [ :index | 

bindings add: (index "asString""asTwoCharacterString" -> (self basicAt: index)) ].

((self basicSize - 20) max: 22) to: (self basicSize) do: [ :index | "self haltIf: [ index = 99 ]."

bindings add: (index "asString" -> (self basicAt: index)) ].

bindings

addAll: ((self class allSlots 

collect: [ :slot | slot name -> (slot read: self) ]) sort asOrderedCollection).

^ bindings

cheers -ben

There is very little meaning behind the number.

The previous inspector showed the first 100 and the last 10 elements. 100 is anyway too large for a quick inspection, so we picked another number. I wanted 42 but that was still large, so we are now at 21.

Doru

> On Feb 3, 2017, at 4:20 PM, Andrei Chis <[hidden email]> wrote:
>
> Yes these numbers should be refactored
> For collections only the first and the last 21 elements are displayed in the Raw view. Don't remember why 21.
>
> Cheers,
> Andrei
>
> On Fri, Feb 3, 2017 at 3:13 AM, Ben Coman <[hidden email]> wrote:
> Just curious what the magic numbers here relate to...
> and can they be factored out to a meaningful method name?
>
> Context>>gtInspectorVariableValuePairs
>       "This is a helper method that returns a collection of
>               variable_name -> value
>       for the current object.
>       Subclasses can override it to specialize what appears in the variables presentation"
>       | bindings |
>       bindings := OrderedCollection new.
>
>       1 to: (self basicSize min: 21) do: [ :index |
>               bindings add: (index "asString""asTwoCharacterString" -> (self basicAt: index)) ].
>
>       ((self basicSize - 20) max: 22) to: (self basicSize) do: [ :index | "self haltIf: [ index = 99 ]."
>               bindings add: (index "asString" -> (self basicAt: index)) ].
>
>       bindings
>               addAll: ((self class allSlots
>                                       collect: [ :slot | slot name -> (slot read: self) ]) sort asOrderedCollection).
>       ^ bindings
>
>
> cheers -ben
>

--
www.tudorgirba.com
www.feenk.com

"Beauty is where we see it."

> On Fri, Feb 3, 2017 at 3:13 AM, Ben Coman <[hidden email]> wrote:
>>
>> Just curious what the magic numbers here relate to...
>> and can they be factored out to a meaningful method name?
>>
>> Context>>gtInspectorVariableValuePairs
>> "This is a helper method that returns a collection of
>> variable_name -> value
>> for the current object.
>> Subclasses can override it to specialize what appears in the variables
>> presentation"
>> | bindings |
>> bindings := OrderedCollection new.
>> 1 to: (self basicSize min: 21) do: [ :index |
>> bindings add: (index "asString""asTwoCharacterString" -> (self basicAt:
>> index)) ].
>> ((self basicSize - 20) max: 22) to: (self basicSize) do: [ :index | "self
>> haltIf: [ index = 99 ]."
>> bindings add: (index "asString" -> (self basicAt: index)) ].
>> bindings
>> addAll: ((self class allSlots
>> collect: [ :slot | slot name -> (slot read: self) ]) sort
>> asOrderedCollection).
>> ^ bindings
>>
>>
>> cheers -ben
>
>

On Fri, Feb 3, 2017 at 11:20 PM, Andrei Chis <[hidden email]> wrote:
> Yes these numbers should be refactored
> For collections only the first and the last 21 elements are displayed in the
> Raw view. Don't remember why 21.
>
> Cheers,
> Andrei
>

Ahhh. Nice to know.  Here I was thinking it was based on some intrinsic restriction on the number of variables or something.

I'm a fan of overusing redundant variables for documenting purposes...

Object>>gtInspectorVariableValuePairs

| indexableDisplayLimit bottom topLimit bottomLimit bindings |

indexableDisplayLimit := 21.

top := 1.

bottom := self basicSize.

topLimit := bottom min: indexableDisplayLimit.

bottomLimit := (bottom - indexableDisplayLimit) max: indexableDisplayLimit.

bindings := OrderedCollection new.

"Return top and bottom of indexable elements"

top to: topLimit do: [ :index | bindings add: (index -> (self basicAt: index)) ]. bottomLimit + 1 to: bottom do: [ :index | bindings add: (index -> (self basicAt: index)) ].  

"Return named variables"

bindings

addAll: ((self class allSlots 

collect: [ :slot | slot name -> (slot read: self) ]) sort asOrderedCollection).

^ bindings

If this looks reasonable I'll do up a slice.

Perhaps defining "top" is overkill - but it does read nice below that.  

btw, in general I understand that some smart optimising compilers will substitute 1 for "top" directly since its assigned a literal and not reassigned before its use.  I notice from the bytecode this doesn't happen here.  Is there some intrinsic difficulty in our domain stopping this to happen, or its just not been a priority.  I guess while stepping through the debugger "top" a user might set a new value for "top" and reverting the substitution of "1" for "top" needs the sort of facility that Sista will provide??

On Sat, Feb 4, 2017 at 12:08 AM, Tudor Girba <[hidden email]> wrote:

There is very little meaning behind the number.

The previous inspector showed the first 100 and the last 10 elements. 100 is anyway too large for a quick inspection, so we picked another number. I wanted 42 but that was still large, so we are now at 21.

Well 21 top and bottom gives you 42, and I know life, the universe and everything depends on that number - so this seems reasonable.

On Sat, Feb 4, 2017 at 12:39 AM, Aliaksei Syrel <[hidden email]> wrote:

They could be extracted to class vars for example TWENTY_ONE := 21. Later if performance is still not good enough they may be changed for example to TWENTY_ONE := 15.

(joke)

You mean something like this...

https://xkcd.com/221/

 

cheers -ben

Done.

Cheers,

Andrei

On Tue, Feb 7, 2017 at 1:35 PM, Ben Coman <[hidden email]> wrote:

On Tue, Feb 7, 2017 at 8:00 PM, Andrei Chis <[hidden email]> wrote:
>
>
> On Sat, Feb 4, 2017 at 4:40 PM, Ben Coman <[hidden email]> wrote:
>>
>>
>> > On Fri, Feb 3, 2017 at 3:13 AM, Ben Coman <[hidden email]> wrote:
>> >>
>> >> Just curious what the magic numbers here relate to...
>> >> and can they be factored out to a meaningful method name?
>> >>
>> >> Context>>gtInspectorVariableValuePairs
>> >> "This is a helper method that returns a collection of
>> >> variable_name -> value
>> >> for the current object.
>> >> Subclasses can override it to specialize what appears in the variables
>> >> presentation"
>> >> | bindings |
>> >> bindings := OrderedCollection new.
>> >> 1 to: (self basicSize min: 21) do: [ :index |
>> >> bindings add: (index "asString""asTwoCharacterString" -> (self basicAt:
>> >> index)) ].
>> >> ((self basicSize - 20) max: 22) to: (self basicSize) do: [ :index |
>> >> "self
>> >> haltIf: [ index = 99 ]."
>> >> bindings add: (index "asString" -> (self basicAt: index)) ].
>> >> bindings
>> >> addAll: ((self class allSlots
>> >> collect: [ :slot | slot name -> (slot read: self) ]) sort
>> >> asOrderedCollection).
>> >> ^ bindings
>> >>
>> >>
>> >> cheers -ben
>> >
>> >
>>
>> On Fri, Feb 3, 2017 at 11:20 PM, Andrei Chis <[hidden email]>
>> wrote:
>> > Yes these numbers should be refactored
>> > For collections only the first and the last 21 elements are displayed in
>> > the
>> > Raw view. Don't remember why 21.
>> >
>> > Cheers,
>> > Andrei
>> >
>>
>> Ahhh. Nice to know.  Here I was thinking it was based on some intrinsic
>> restriction on the number of variables or something.
>>
>> I'm a fan of overusing redundant variables for documenting purposes...
>>
>> Object>>gtInspectorVariableValuePairs
>> | indexableDisplayLimit bottom topLimit bottomLimit bindings |
>>
>> indexableDisplayLimit := 21.
>> top := 1.
>> bottom := self basicSize.
>> topLimit := bottom min: indexableDisplayLimit.
>> bottomLimit := (bottom - indexableDisplayLimit) max: indexableDisplayLimit.
>>
>> bindings := OrderedCollection new.
>> "Return top and bottom of indexable elements"
>> top to: topLimit do: [ :index | bindings add: (index -> (self basicAt:
>> index)) ]. bottomLimit + 1 to: bottom do: [ :index | bindings add: (index ->
>> (self basicAt: index)) ].
>>
>> "Return named variables"
>> bindings
>> addAll: ((self class allSlots
>> collect: [ :slot | slot name -> (slot read: self) ]) sort
>> asOrderedCollection).
>> ^ bindings
>>
>> If this looks reasonable I'll do up a slice.
>
>
> Looks good. I'll commit this to the inspector repo and will be picked by the
> next integration.

Thanks Andre.

Very minor thing if its not too late. Looking at it again I'd actually
rearrange these two lines like this...
  topLimit        := indexableDisplayLimit min: bottom.
  bottomLimit := indexableDisplayLimit max: (bottom - indexableDisplayLimit) .

But don't worry if you've already done the first way.
cheers -ben

>
> Cheers,
> Andrei
>
>>
>>
>> Perhaps defining "top" is overkill - but it does read nice below that.
>> btw, in general I understand that some smart optimising compilers will
>> substitute 1 for "top" directly since its assigned a literal and not
>> reassigned before its use.  I notice from the bytecode this doesn't happen
>> here.  Is there some intrinsic difficulty in our domain stopping this to
>> happen, or its just not been a priority.  I guess while stepping through the
>> debugger "top" a user might set a new value for "top" and reverting the
>> substitution of "1" for "top" needs the sort of facility that Sista will
>> provide??
>>
>>
>> On Sat, Feb 4, 2017 at 12:08 AM, Tudor Girba <[hidden email]> wrote:
>>>
>>> There is very little meaning behind the number.
>>>
>>> The previous inspector showed the first 100 and the last 10 elements. 100
>>> is anyway too large for a quick inspection, so we picked another number. I
>>> wanted 42 but that was still large, so we are now at 21.
>>
>>
>> Well 21 top and bottom gives you 42, and I know life, the universe and
>> everything depends on that number - so this seems reasonable.
>>
>>
>> On Sat, Feb 4, 2017 at 12:39 AM, Aliaksei Syrel <[hidden email]>
>> wrote:
>>>
>>> They could be extracted to class vars for example TWENTY_ONE := 21. Later
>>> if performance is still not good enough they may be changed for example to
>>> TWENTY_ONE := 15.
>>> (joke)
>>
>>
>> You mean something like this...
>> https://xkcd.com/221/
>>
>>
>> cheers -ben
>
>