Fix: Implement = and hash#17
Conversation
| ] | ||
|
|
||
| { #category : 'tests' } | ||
| CTQueueTest >> testEnqueueUnless [ |
There was a problem hiding this comment.
Can you split this test? Normally tests should be descriptive
| CTQueue >> = anObject [ | ||
|
|
||
| self == anObject ifTrue: [ ^ true ]. | ||
| self species == anObject species ifFalse: [ ^ false ]. |
There was a problem hiding this comment.
why the species comparison is necessary here?
There was a problem hiding this comment.
I was initially mimicking how standard Pharo collections use species for their equality checks. But I realize now that since CTQueue inherits directly from Object, using species is too loose and could cause unexpected behavior if the queue is subclassed. i pushed an updated for that
| { #category : 'comparing' } | ||
| CTQueue >> = anObject [ | ||
|
|
||
| self == anObject ifTrue: [ ^ true ]. |
There was a problem hiding this comment.
This is wrong. First you should compare if they have the same class
There was a problem hiding this comment.
i pushed an updated for that
| CTQueue >> hash [ | ||
|
|
||
| | hashValue | | ||
| hashValue := self species hash. |
|
|
||
| | hashValue | | ||
| hashValue := self species hash. | ||
| self do: [ :each | hashValue := (hashValue + each hash) hashMultiply ]. |
There was a problem hiding this comment.
the hash function looks weird. We need to test it to see the collisions, etc. We need some benchmarks for that. They do not need to be in the tests but we should make sure that is a good hash function
2 participants
Fix #16.
Previously,
CTQueueinheritedObject's default identity comparison, meaning two queues with the exact same elements were not considered equal.Changes made:
#=inCTQueueto compare species, size, and internal elements.#hashinCTQueueusing the standard collection hash multiplier.testEqualitytoCTQueueTestto verify that identical queues evaluate totrueand their hashes match.All tests are Green now