[Pharo-dev] magic numbers in gtInspectorVariableValuePairs

Andrei Chis chisvasileandrei at gmail.com
Tue Feb 7 09:46:30 EST 2017


Done.

Cheers,
Andrei

On Tue, Feb 7, 2017 at 1:35 PM, Ben Coman <btc at openinworld.com> wrote:

> On Tue, Feb 7, 2017 at 8:00 PM, Andrei Chis <chisvasileandrei at gmail.com>
> wrote:
> >
> >
> > 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.
>
> 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 <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/adf0f04b/attachment.html>


More information about the Pharo-dev mailing list