Rectangle>>merging: does not accepts Sets anymore

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Rectangle>>merging: does not accepts Sets anymore

Stéphane Rollandin
Hello all,

I just noticed that Rectangle class>>#merging: now (well, since summer
2020) uses #first and so does not allow its argument to be a Set
anymore. Is that the intended behavior?

Stef

Reply | Threaded
Open this post in threaded view
|

Re: Rectangle>>merging: does not accepts Sets anymore

David T. Lewis
On Sat, Feb 13, 2021 at 12:57:15PM +0100, St??phane Rollandin wrote:
> Hello all,
>
> I just noticed that Rectangle class>>#merging: now (well, since summer
> 2020) uses #first and so does not allow its argument to be a Set
> anymore. Is that the intended behavior?
>

Hi Stef,

The change was introduced here:

  Name: Graphics-kfr.436
  Author: kfr
  Time: 22 August 2020, 11:47:40.557622 am
 
  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
 
  Updated to  use #first to get the first element and #allButFirstDo:
  to iterate over the rest.  (Suggested by Levente Uzonyi)

Two methods are affected, #merging: and #encompassing:

I don't have an easy way to verify, but it looks like adding #asArray
might resolve the issue for Sets (e.g. listOfRects asArray in #merging:).
Can you check and see if that is what is needed?

I do not know if this would be a proper fix but it would at least
clarify the problem.

Thanks,

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Rectangle>>merging: does not accepts Sets anymore

Nicolas Cellier
We could use anyOne instead of first; and do: instead of allButFirstDo:
Iterating twice on anyOne is really benign...

Le sam. 13 févr. 2021 à 16:41, David T. Lewis <[hidden email]> a écrit :

>
> On Sat, Feb 13, 2021 at 12:57:15PM +0100, St??phane Rollandin wrote:
> > Hello all,
> >
> > I just noticed that Rectangle class>>#merging: now (well, since summer
> > 2020) uses #first and so does not allow its argument to be a Set
> > anymore. Is that the intended behavior?
> >
>
> Hi Stef,
>
> The change was introduced here:
>
>   Name: Graphics-kfr.436
>   Author: kfr
>   Time: 22 August 2020, 11:47:40.557622 am
>
>   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
>
>   Updated to  use #first to get the first element and #allButFirstDo:
>   to iterate over the rest.  (Suggested by Levente Uzonyi)
>
> Two methods are affected, #merging: and #encompassing:
>
> I don't have an easy way to verify, but it looks like adding #asArray
> might resolve the issue for Sets (e.g. listOfRects asArray in #merging:).
> Can you check and see if that is what is needed?
>
> I do not know if this would be a proper fix but it would at least
> clarify the problem.
>
> Thanks,
>
> Dave
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Rectangle>>merging: does not accepts Sets anymore

Stéphane Rollandin
>> I don't have an easy way to verify, but it looks like adding #asArray
>> might resolve the issue for Sets (e.g. listOfRects asArray in #merging:).

Yes it would, but at a cost that I would better avoid (both in term of
performance and GC), since #merging: is a pretty fundamental method for
computational geometry.

Best,

Stef

Reply | Threaded
Open this post in threaded view
|

Re: Rectangle>>merging: does not accepts Sets anymore

Chris Muller-3
On Sat, Feb 13, 2021 at 10:50 AM Stéphane Rollandin <[hidden email]> wrote:
>> I don't have an easy way to verify, but it looks like adding #asArray
>> might resolve the issue for Sets (e.g. listOfRects asArray in #merging:).

Yes it would, but at a cost that I would better avoid (both in term of
performance and GC), since #merging: is a pretty fundamental method for 
computational geometry.

In which case you should make use of #quickMerge: then.


Reply | Threaded
Open this post in threaded view
|

Re: Rectangle>>merging: does not accepts Sets anymore

Stéphane Rollandin
> In which case you should make use of #quickMerge: then.

Thanks! I was not aware of this one.

Stef

Reply | Threaded
Open this post in threaded view
|

Re: Rectangle>>merging: does not accepts Sets anymore

Levente Uzonyi
In reply to this post by Chris Muller-3
On Mon, 15 Feb 2021, Chris Muller wrote:

> On Sat, Feb 13, 2021 at 10:50 AM Stéphane Rollandin <[hidden email]> wrote:
>       >> I don't have an easy way to verify, but it looks like adding #asArray
>       >> might resolve the issue for Sets (e.g. listOfRects asArray in #merging:).
>
>       Yes it would, but at a cost that I would better avoid (both in term of
>       performance and GC), since #merging: is a pretty fundamental method for 
>
>       computational geometry.
>
>
> In which case you should make use of #quickMerge: then.
>
>
I don't think that's a good idea. It'll still create lots of garbage when
merging more than two rectangles.


Levente

Reply | Threaded
Open this post in threaded view
|

Re: Rectangle>>merging: does not accepts Sets anymore

codefrau
In reply to this post by Nicolas Cellier
On Sat, Feb 13, 2021 at 7:56 AM Nicolas Cellier <[hidden email]> wrote:
We could use anyOne instead of first; and do: instead of allButFirstDo:
Iterating twice on anyOne is really benign...

+1

- Vanessa -