Skip to content

Conversation

@FiV0
Copy link

@FiV0 FiV0 commented Dec 16, 2022

This is how I would add a custom accumulator to get-range.

I would not use something like a flatland/map as you mentioned in #29. I would even consider making get-range always return a sequential structure by default (which breaks downstream but I don't know if that is an issue for you). We can discuss more how you think it is best approached.

(transient empty-coll)
(ftr/get-range tr rg))
persistent!))))))

Copy link
Owner

Choose a reason for hiding this comment

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

The persistent / transient is a simple enough change (though I find the use of the -> to be overkill here)

Re: The collection option, let me think about how I want to return an ordered range.

A map makes sense to me since the value cannot and should not be separated from the key. But this does break ordering which is an important property to maintain. I still think that ordered-map is the right way to go here.

Copy link
Author

@FiV0 FiV0 Dec 16, 2022

Choose a reason for hiding this comment

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

Personally I would always consume (transform or use otherwise in my app) everything that is returned by get-range so I find the building up of a hash-map or other map unnecessary. Something like '([k1 v1] [k2 v2] ...)seems sufficient and if somebody needs a hash-map they can always (into {} res). Especially if you want to write performant code this is just overhead.

I will look at your other comments some time this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants