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

Feature request: shift_insert #173

Closed
TethysSvensson opened this issue Feb 21, 2021 · 3 comments · Fixed by #310
Closed

Feature request: shift_insert #173

TethysSvensson opened this issue Feb 21, 2021 · 3 comments · Fixed by #310

Comments

@TethysSvensson
Copy link

I would like to be able to insert an element at a particular index in the indexmap (knowning full well, that this will cause O(n) move operations).

@cuviper
Copy link
Member

cuviper commented Feb 26, 2021

I suppose if the item already exists at any index, you want it moved to the new location?

Rough sketch:

    pub fn insert_at(&mut self, index: usize, key: K, value: V) -> Option<V> {
        let (i, old) = self.insert_full(key, value);
        if i < index {
            // TODO: adjust `indices` too
            self.entries[i..=index].rotate_left(1);
        } else if i > index {
            // TODO: adjust `indices` too
            self.entries[index..=i].rotate_right(1);
        }
        old
    }

The crude way to update indices is IndexMapCore::rebuild_hash_table -- reinserting everything, but at least we have saved hashes. We can probably do better though, more like the heuristics in shift_remove.

@TethysSvensson
Copy link
Author

TethysSvensson commented Feb 26, 2021

I suppose if the item already exists at any index, you want it moved to the new location?

For my particular use-case I wish to keep the item at the current index, but I need to modify it as well. However, performance is not very critical at all, so I am completely okay with handling this by doing a second lookup.

I think you are right, that the most reasonable behavior would be to move the item it.

I guess the way to completely support my use-case would be to add shift_insert methods to both VacantEntry and OccupiedEntry, but I am not sure if this is worth the added complexity. Then I guess you could also add a or_shift_insert to Entry.

cuviper added a commit to cuviper/indexmap that referenced this issue Feb 27, 2021
This moves the position of a key-value pair from one index to another by
shifting all other pairs in-between, making this an O(n) operation.

This could be used as a building-block for other operations, like indexmap-rs#173
which wants to insert at a particular index. You can `insert_full` to
insert it _somewhere_, then choose whether to `move_index` depending on
whether you want to also change pre-existing entries.
@cuviper
Copy link
Member

cuviper commented Feb 27, 2021

Please see #176 for a move_index method that I think would be useful to you. That does the hard part of updating indices, and then we can possibly build new insertion or entry methods on top of that.

cuviper added a commit to cuviper/indexmap that referenced this issue Feb 27, 2021
This moves the position of a key-value pair from one index to another by
shifting all other pairs in-between, making this an O(n) operation.

This could be used as a building-block for other operations, like indexmap-rs#173
which wants to insert at a particular index. You can `insert_full` to
insert it _somewhere_, then choose whether to `move_index` depending on
whether you want to also change pre-existing entries.
cuviper added a commit to cuviper/indexmap that referenced this issue Feb 27, 2021
This moves the position of a key-value pair from one index to another by
shifting all other pairs in-between, making this an O(n) operation.

This could be used as a building-block for other operations, like indexmap-rs#173
which wants to insert at a particular index. You can `insert_full` to
insert it _somewhere_, then choose whether to `move_index` depending on
whether you want to also change pre-existing entries.
cuviper added a commit to cuviper/indexmap that referenced this issue May 13, 2022
This moves the position of a key-value pair from one index to another by
shifting all other pairs in-between, making this an O(n) operation.

This could be used as a building-block for other operations, like indexmap-rs#173
which wants to insert at a particular index. You can `insert_full` to
insert it _somewhere_, then choose whether to `move_index` depending on
whether you want to also change pre-existing entries.
cuviper added a commit to cuviper/indexmap that referenced this issue May 13, 2022
This moves the position of a key-value pair from one index to another by
shifting all other pairs in-between, making this an O(n) operation.

This could be used as a building-block for other operations, like indexmap-rs#173
which wants to insert at a particular index. You can `insert_full` to
insert it _somewhere_, then choose whether to `move_index` depending on
whether you want to also change pre-existing entries.
cuviper added a commit to cuviper/indexmap that referenced this issue Jun 16, 2022
This moves the position of a key-value pair from one index to another by
shifting all other pairs in-between, making this an O(n) operation.

This could be used as a building-block for other operations, like indexmap-rs#173
which wants to insert at a particular index. You can `insert_full` to
insert it _somewhere_, then choose whether to `move_index` depending on
whether you want to also change pre-existing entries.

(cherry picked from commit 54a48d2)
@cuviper cuviper linked a pull request Feb 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants