Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SoilIndexedDictionary: implement size to restore values #544

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

MarcusDenker
Copy link
Contributor

this uses the #do of the SoilIndexedDictionary in size, this way it is can restore values correctly on the level of the SoilIndexedDictionary.

fixes #527

…s can restore values correctly on the level of the SoilIndexedDictionary.

fixes #527
@MarcusDenker MarcusDenker requested a review from noha December 8, 2023 09:34
@MarcusDenker MarcusDenker changed the title SoilIndexedDictionary: implement using the local do: so it correctly restores values SoilIndexedDictionary: implement size to restore values Dec 8, 2023
Copy link
Contributor

@noha noha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss!

"We should just call size on index, but the do: here can restore values"
| sum |
sum := 0.
self do: [ :each | sum := sum + 1 ].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not scale at all. Not sure how we do it until now but doing a linear scan counting is not the way to go

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we do already do the same linear scam on the index...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SoilBasicSkipList>>#size
	"We iterate over all elements to get the size. Slow!"
	^ self newIterator size 

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was already #226, I updated it.

@noha noha merged commit 1b82903 into main Dec 8, 2023
2 checks passed
@MarcusDenker MarcusDenker deleted the 527-SoilIndexedDictionarysize-and-restored-values branch December 8, 2023 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

SoilIndexedDictionary>>#size and restored values
2 participants