Skip to content

Commit

Permalink
Merge pull request #14 from Sajjon/fix_bug_dont_delete_element_if-try…
Browse files Browse the repository at this point in the history
…_update_with_fails

Fix bug, dont delete element of try_update_with fails.
  • Loading branch information
Sajjon authored Dec 22, 2023
2 parents a6299e5 + a4379bb commit 9144399
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 13 deletions.
16 changes: 11 additions & 5 deletions src/vec/identified_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,16 +490,22 @@ where
#[inline]
fn try_update_with<F, Er>(&mut self, id: &I, mut mutate: F) -> Result<bool, Er>
where
F: FnMut(E) -> Result<E, Er>,
F: FnMut(&mut E) -> Result<E, Er>,
{
if !self.contains_id(id) {
return Ok(false);
}
let mut existing = self.elements.remove(id).expect("Element for existing id");
mutate(existing).map(|updated| {
self.elements.insert(id.clone(), updated);
true
})

mutate(&mut existing)
.map(|updated| {
self.elements.insert(id.clone(), updated);
true
})
.map_err(|e| {
self.elements.insert(id.clone(), existing);
e
})
}

/// Try to update the given element to the `identified_vec` if a element with the same ID is already present.
Expand Down
2 changes: 1 addition & 1 deletion src/vec/is_identified_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ where

fn try_update_with<F, Er>(&mut self, id: &ID, mutate: F) -> Result<bool, Er>
where
F: FnMut(Element) -> Result<Element, Er>;
F: FnMut(&mut Element) -> Result<Element, Er>;

/// Insert a new member to this identified_vec at the specified index, if the identified_vec doesn't already contain
/// it.
Expand Down
2 changes: 1 addition & 1 deletion src/vec_of/is_identified_vec_of_via.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ where
mut mutate: F,
) -> Result<bool, Er>
where
F: FnMut(Element) -> Result<Element, Er>,
F: FnMut(&mut Element) -> Result<Element, Er>,
{
self.via_mut().try_update_with(id, mutate)
}
Expand Down
39 changes: 33 additions & 6 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,9 +410,8 @@ fn try_update_with_contains() {
sut.append(User::new(2, "Blob, Jr."));
assert_eq!(
sut.try_update_with(&2, |x| {
let mut y = x;
*y.name.get_mut() = "Junior, Jr.".to_string();
Result::<User, ()>::Ok(y)
*x.name.get_mut() = "Junior, Jr.".to_string();
Result::<User, ()>::Ok(x.clone())
}),
Ok(true)
);
Expand All @@ -425,15 +424,43 @@ fn try_update_with_not_contains() {
sut.append(User::new(2, "Blob, Jr."));
assert_eq!(
sut.try_update_with(&999, |x| {
let mut y = x;
*y.name.get_mut() = "Will never happen.".to_string();
Result::<User, ()>::Ok(y)
*x.name.get_mut() = "Will never happen.".to_string();
Result::<User, ()>::Ok(x.clone())
}),
Ok(false)
);
assert_eq!(sut.items(), [User::new(2, "Blob, Jr.")]);
}

#[test]
fn try_update_with_failure_does_not_delete_element() {
let mut sut = Users::new();
sut.append(User::new(1, "Blob."));
sut.append(User::new(2, "Blob, Jr."));
sut.append(User::new(3, "Blob, Sr."));

assert_eq!(
sut.items(),
[
User::new(1, "Blob."),
User::new(2, "Blob, Jr."),
User::new(3, "Blob, Sr.")
]
);

assert_eq!(sut.try_update_with(&2, |_| { Err(false) }), Err(false));

// remains unchanged
assert_eq!(
sut.items(),
[
User::new(1, "Blob."),
User::new(2, "Blob, Jr."),
User::new(3, "Blob, Sr.")
]
);
}

#[test]
#[should_panic(expected = "Expected element at index {index}")]
fn update_at_expect_panic_unknown_index() {
Expand Down

0 comments on commit 9144399

Please sign in to comment.