<div dir="ltr"><div>Hi,</div><div><br></div><div>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.</div><div>Also looping with allButFirstDo: is slower than the original method.</div><div><br></div><div>This seems to be fastest:</div><div><br></div><div>Rectangle>>merging: listOfRects <br>    "A number of callers of merge: should use this method."<br>     | minX minY maxX maxY |<br>       minX := listOfRects first topLeft x. <br> minY := listOfRects first topLeft y.<br>  maxX := listOfRects first bottomRight x. <br>     maxY := listOfRects first bottomRight y.<br>      listOfRects do: [:r | minX := minX min: r topLeft x. <br></div><div>             minY := minY min: r topLeft y.<br>             maxX := maxX max: r bottomRight x. <br></div><div>             maxY := maxY max: r bottomRight y].<br>      ^self origin: minX@minY corner: maxX@maxY<br>     </div><div><br></div><div>Best,</div><div>Karl<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jul 11, 2020 at 3:49 PM Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu">leves@caesar.elte.hu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Karl,<br>
<br>
While both changes look right, they modify the behavior of those methods <br>
when their argument is an empty list.<br>
Instead of raising an error, they'll return a Rectangle with both fields <br>
set to nil.<br>
If the argument is really a list - aka SequenceableCollection, I suggest <br>
using more specific methods instead of #do: to process the list in order <br>
to trigger the error as soon as possible.<br>
For example, use #first to get the first element and #allButFirstDo: to <br>
iterate over the rest. That would simplify the method as well by making <br>
the nil checks unnecessary.<br>
<br>
<br>
Levente<br>
<br>
On Sat, 11 Jul 2020, <a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a> wrote:<br>
<br>
> A new version of Graphics was added to project The Inbox:<br>
> <a href="http://source.squeak.org/inbox/Graphics-kfr.434.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/inbox/Graphics-kfr.434.mcz</a><br>
><br>
> ==================== Summary ====================<br>
><br>
> Name: Graphics-kfr.434<br>
> Author: kfr<br>
> Time: 11 July 2020, 10:18:06.272175 am<br>
> UUID: 0e0125f6-3040-6247-9e5a-43bfd1896f84<br>
> Ancestors: Graphics-mt.433<br>
><br>
> 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<br>
><br>
> =============== Diff against Graphics-mt.433 ===============<br>
><br>
> Item was changed:<br>
>  ----- Method: Rectangle class>>encompassing: (in category 'instance creation') -----<br>
>  encompassing: listOfPoints<br>
>       "A number of callers of encompass: should use this method."<br>
>       | topLeft bottomRight |<br>
>       topLeft := bottomRight := nil.<br>
>       listOfPoints do:<br>
>               [:p | topLeft == nil<br>
>                       ifTrue: [topLeft := bottomRight := p]<br>
>                       ifFalse: [topLeft := topLeft min: p.<br>
>                                       bottomRight := bottomRight max: p]].<br>
> +     ^self origin: topLeft corner: bottomRight!<br>
> -     ^ topLeft corner: bottomRight!<br>
><br>
> Item was changed:<br>
>  ----- Method: Rectangle class>>merging: (in category 'instance creation') -----<br>
>  merging: listOfRects<br>
>       "A number of callers of merge: should use this method."<br>
>       | minX minY maxX maxY |<br>
>       listOfRects<br>
>               do: [:r | minX<br>
>                               ifNil: [minX := r topLeft x. minY := r topLeft y.<br>
>                                       maxX := r bottomRight x. maxY := r bottomRight y]<br>
>                               ifNotNil: [minX := minX min: r topLeft x. minY := minY min: r topLeft y.<br>
>                                       maxX := maxX max: r bottomRight x. maxY := maxY max: r bottomRight y]].<br>
> +     ^ self origin:minX@minY corner: maxX@maxY!<br>
> -     ^ minX@minY corner: maxX@maxY!<br>
<br>
</blockquote></div>