Re: [Pharo-project] Issue 2841 in pharo: Heap equality should be transitive
Comment #1 on issue 2841 by nicolas....@gmail.com: 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 ?
Comment #2 on issue 2841 by nicolas....@gmail.com: 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.
Comment #3 on issue 2841 by nicolas....@gmail.com: 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.
Updates: Status: FixProposed Comment #4 on issue 2841 by nicolas....@gmail.com: Heap equality should be transitive http://code.google.com/p/pharo/issues/detail?id=2841 (No comment was entered for this change.)
participants (1)
-
pharo＠googlecode.com