Skip to content

Commit

Permalink
🐛 (MapExt): Fix a bug where when k1 exists and equals k2, the correct…
Browse files Browse the repository at this point in the history
… behavior should be to do nothing and return `Ok(())`
  • Loading branch information
czy-29 committed Nov 22, 2024
1 parent ca6a37a commit 78fd143
Showing 1 changed file with 29 additions and 1 deletion.
30 changes: 29 additions & 1 deletion src/collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ pub trait MapExt<K, Q: ?Sized = K> {
///
/// If k1 does not exist, return `Err(ReplaceKeyErr::OldKeyNotExist)`.
///
/// If k1 exists and k2 also exists, return `Err(ReplaceKeyErr::NewKeyOccupied)`.
/// Otherwise, if k2 and k1 are equal, do nothing and return `Ok(())`.
///
/// Otherwise, if k2 also exists, return `Err(ReplaceKeyErr::NewKeyOccupied)`.
///
/// Otherwise, return `Ok(())` after the replacement is completed.
fn replace_key(&mut self, k1: &Q, k2: K) -> Result<(), ReplaceKeyErr>
Expand All @@ -44,6 +46,10 @@ where
return Err(ReplaceKeyErr::OldKeyNotExist);
}

if k1 == k2.borrow() {
return Ok(());
}

if self.contains_key(k2.borrow()) {
return Err(ReplaceKeyErr::NewKeyOccupied);
}
Expand All @@ -64,6 +70,10 @@ where
return Err(ReplaceKeyErr::OldKeyNotExist);
}

if k1 == k2.borrow() {
return Ok(());
}

if self.contains_key(k2.borrow()) {
return Err(ReplaceKeyErr::NewKeyOccupied);
}
Expand Down Expand Up @@ -91,10 +101,19 @@ mod tests {
map.replace_key("k3", "k2".to_string()),
Err(ReplaceKeyErr::OldKeyNotExist)
);
assert_eq!(
map.replace_key("k3", "k3".to_string()),
Err(ReplaceKeyErr::OldKeyNotExist)
);
assert_eq!(
map.replace_key("k3", "k4".to_string()),
Err(ReplaceKeyErr::OldKeyNotExist)
);

let cloned = map.clone();
assert_eq!(map.replace_key("k1", "k1".to_string()), Ok(()));
assert_eq!(map, cloned);

assert_eq!(
map.replace_key("k1", "k2".to_string()),
Err(ReplaceKeyErr::NewKeyOccupied)
Expand All @@ -118,10 +137,19 @@ mod tests {
map.replace_key("k3", "k2".to_string()),
Err(ReplaceKeyErr::OldKeyNotExist)
);
assert_eq!(
map.replace_key("k3", "k3".to_string()),
Err(ReplaceKeyErr::OldKeyNotExist)
);
assert_eq!(
map.replace_key("k3", "k4".to_string()),
Err(ReplaceKeyErr::OldKeyNotExist)
);

let cloned = map.clone();
assert_eq!(map.replace_key("k1", "k1".to_string()), Ok(()));
assert_eq!(map, cloned);

assert_eq!(
map.replace_key("k1", "k2".to_string()),
Err(ReplaceKeyErr::NewKeyOccupied)
Expand Down

0 comments on commit 78fd143

Please sign in to comment.