[Pharo-dev] magic numbers in gtInspectorVariableValuePairs

Andrei Chis chisvasileandrei at gmail.com
Tue Feb 7 07:00:23 EST 2017


On Sat, Feb 4, 2017 at 4:40 PM, Ben Coman <btc at openinworld.com> wrote:

>
> > On Fri, Feb 3, 2017 at 3:13 AM, Ben Coman <btc at openinworld.com> 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 <chisvasileandrei at gmail.com>
> 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.

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 <tudor at tudorgirba.com> 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 <alex.syrel at gmail.com>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.pharo.org/pipermail/pharo-dev_lists.pharo.org/attachments/20170207/b061485e/attachment.html>


More information about the Pharo-dev mailing list