Hi,

In Rectangle>>merging: it is quite a bit slower to use points topLeft and
bottomRight, than reading out the min and max of each side.
Also looping with allButFirstDo: is slower than the original method.

This seems to be fastest:

Rectangle>>merging: listOfRects
"A number of callers of merge: should use this method."
| minX minY maxX maxY |
minX := listOfRects first topLeft x.
minY := listOfRects first topLeft y.
maxX := listOfRects first bottomRight x.
maxY := listOfRects first bottomRight y.
listOfRects do: [:r | minX := minX min: r topLeft x.
             minY := minY min: r topLeft y.
             maxX := maxX max: r bottomRight x.
             maxY := maxY max: r bottomRight y].
^self origin: minX at minY corner: maxX at maxY

Best,
Karl

On Sat, Jul 11, 2020 at 3:49 PM Levente Uzonyi <leves at caesar.elte.hu> wrote:

> Hi Karl,
>
> While both changes look right, they modify the behavior of those methods
> when their argument is an empty list.
> Instead of raising an error, they'll return a Rectangle with both fields
> set to nil.
> If the argument is really a list - aka SequenceableCollection, I suggest
> using more specific methods instead of #do: to process the list in order
> to trigger the error as soon as possible.
> For example, use #first to get the first element and #allButFirstDo: to
> iterate over the rest. That would simplify the method as well by making
> the nil checks unnecessary.
>
>
> Levente
>
> On Sat, 11 Jul 2020, commits at source.squeak.org wrote:
>
> > A new version of Graphics was added to project The Inbox:
> > http://source.squeak.org/inbox/Graphics-kfr.434.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Graphics-kfr.434
> > Author: kfr
> > Time: 11 July 2020, 10:18:06.272175 am
> > UUID: 0e0125f6-3040-6247-9e5a-43bfd1896f84
> > Ancestors: Graphics-mt.433
> >
> > I wanted to subclass Rectangle with a instance variable to carry some
> state, but these methodes indirected to Point>>corner: so it broke the
> override and returned a ordinary Rectangle instead of my fancy new subclass
> SuperRectangle
> >
> > =============== Diff against Graphics-mt.433 ===============
> >
> > Item was changed:
> >  ----- Method: Rectangle class>>encompassing: (in category 'instance
> creation') -----
> >  encompassing: listOfPoints
> >       "A number of callers of encompass: should use this method."
> >       | topLeft bottomRight |
> >       topLeft := bottomRight := nil.
> >       listOfPoints do:
> >               [:p | topLeft == nil
> >                       ifTrue: [topLeft := bottomRight := p]
> >                       ifFalse: [topLeft := topLeft min: p.
> >                                       bottomRight := bottomRight max:
> p]].
> > +     ^self origin: topLeft corner: bottomRight!
> > -     ^ topLeft corner: bottomRight!
> >
> > Item was changed:
> >  ----- Method: Rectangle class>>merging: (in category 'instance
> creation') -----
> >  merging: listOfRects
> >       "A number of callers of merge: should use this method."
> >       | minX minY maxX maxY |
> >       listOfRects
> >               do: [:r | minX
> >                               ifNil: [minX := r topLeft x. minY := r
> topLeft y.
> >                                       maxX := r bottomRight x. maxY := r
> bottomRight y]
> >                               ifNotNil: [minX := minX min: r topLeft x.
> minY := minY min: r topLeft y.
> >                                       maxX := maxX max: r bottomRight x.
> maxY := maxY max: r bottomRight y]].
> > +     ^ self origin:minX at minY corner: maxX at maxY!
> > -     ^ minX at minY corner: maxX at maxY!
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200712/0db04dbc/attachment.html>