Re: Issue 2841 in pharo: Heap equality should be transitive

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

Re: Issue 2841 in pharo: Heap equality should be transitive

pharo

Comment #1 on issue 2841 by [hidden email]: Heap equality should be  
transitive
http://code.google.com/p/pharo/issues/detail?id=2841

If I download the code from Squeak trunk, it will break a lot of tests in  
HeapTest.
(only due to the fact that I remove #species).
I believe that assumptions made about Heap are not right.

I wonder why the Heap should be considered as Sequenceable.
Elements are partially sorted and the structure is more of a binary tree.
Elements are not fully sorted, thus the order:
- is not the order of the addition of elements,
- is not independent of the order of addition.
In fact, only the Heap head (first) is meaningfull.

Thus testing some properties like #beginsWith: #endsWith: #at: #at:put:  
#reversed #copyFrom:to: etc... seems very fragile to me.
Of course, that would mean moving the Heap one level up, directly under  
Collection, in order to cease undue inheritance of sequenceable API.

What do you think ?


Reply | Threaded
Open this post in threaded view
|

Re: Issue 2841 in pharo: Heap equality should be transitive

pharo

Comment #2 on issue 2841 by [hidden email]: Heap equality should be  
transitive
http://code.google.com/p/pharo/issues/detail?id=2841

I also think it is a mistake to inherit from TSortable.
TSortable implements a mergeSort algorithm.
Heap has it's own idea on how it should sort, so this inheritance does not  
have much sense.
Just using (Heap asArray sort) shall be enough.



Reply | Threaded
Open this post in threaded view
|

Re: Issue 2841 in pharo: Heap equality should be transitive

pharo

Comment #3 on issue 2841 by [hidden email]: Heap equality should be  
transitive
http://code.google.com/p/pharo/issues/detail?id=2841

SLICE-Issue-2841-HeapEqualityShouldBeTransitive-ButHeapShouldNotBeSequenceable-nice.1
Author: nice

I merged trunk changes and also moved Heap off SequenceableCollection  
hierarchy.
I removed plenty of Sequenceable assumptions from HeapTests, and also some  
assumptions about equality in IntervalTests.

I forbid usage of #at: #at:put: #removeAt:, that's not a legitimate API for  
a Heap.
Only #first #add: #removeFirst #remove: shall be used.

Of course, one thing remain to be done: move Heap to another package than  
Collections-Sequenceable.
I didn't do it for at least two reasons:
1) I down't know how to classify a Heap, it's Ordered (partially), not  
Unordered, and not Sequenceable.
2) Heap must not be temporarily removed because used in Delay and World  
alarms, so package load order will be very important.
I let the experts of smooth upgrading do it.


Reply | Threaded
Open this post in threaded view
|

Re: Issue 2841 in pharo: Heap equality should be transitive

pharo
Updates:
        Status: FixProposed

Comment #4 on issue 2841 by [hidden email]: Heap equality should be  
transitive
http://code.google.com/p/pharo/issues/detail?id=2841

(No comment was entered for this change.)