From 690c172cf6a4d1fae07db0ad7de56b5197563267 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Thu, 11 Jan 2024 21:47:35 +0100 Subject: [PATCH] Chaneg artist new/delete to add/remove --- src/bin/musichoard-edit.rs | 18 +++---- src/lib.rs | 102 ++++++++++++++++++------------------- 2 files changed, 58 insertions(+), 62 deletions(-) diff --git a/src/bin/musichoard-edit.rs b/src/bin/musichoard-edit.rs index b4261ce..5d20230 100644 --- a/src/bin/musichoard-edit.rs +++ b/src/bin/musichoard-edit.rs @@ -42,9 +42,9 @@ impl Category { #[derive(StructOpt, Debug)] enum ArtistCommand { #[structopt(about = "Add a new artist to the collection")] - New(ArtistValue), - #[structopt(about = "Delete an artist from the collection")] - Delete(ArtistValue), + Add(ArtistValue), + #[structopt(about = "Remove an artist from the collection")] + Remove(ArtistValue), #[structopt(name = "musicbrainz", about = "Edit the MusicBrainz URL of an artist")] MusicBrainz(UrlCommand), #[structopt( @@ -132,15 +132,11 @@ macro_rules! multi_url_command_dispatch { impl ArtistCommand { fn handle(self, music_hoard: &mut MH) { match self { - ArtistCommand::New(artist_value) => { - music_hoard - .new_artist(ArtistId::new(artist_value.artist)) - .expect("failed to add new artist"); + ArtistCommand::Add(artist_value) => { + music_hoard.add_artist(ArtistId::new(artist_value.artist)); } - ArtistCommand::Delete(artist_value) => { - music_hoard - .delete_artist(ArtistId::new(artist_value.artist)) - .expect("failed to delete artist"); + ArtistCommand::Remove(artist_value) => { + music_hoard.remove_artist(ArtistId::new(artist_value.artist)); } ArtistCommand::MusicBrainz(url_command) => { single_url_command_dispatch!(url_command, music_hoard, musicbrainz) diff --git a/src/lib.rs b/src/lib.rs index 5d95931..718ed82 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -829,38 +829,25 @@ impl MusicHoard { } } - pub fn new_artist>(&mut self, artist_id: ID) -> Result<(), Error> { + pub fn add_artist>(&mut self, artist_id: ID) { let artist_id: ArtistId = artist_id.into(); - if let Ok(artist) = self.get_artist_or_err(&artist_id) { - return Err(Error::CollectionError(format!( - "artist '{}' is already in the collection", - artist.id - ))); + + if self.get_artist(&artist_id).is_none() { + let new_artist = vec![Artist::new(artist_id)]; + + let collection = mem::take(&mut self.collection); + self.collection = Self::merge(collection, new_artist); } - - let new_artist = vec![Artist::new(artist_id)]; - - let collection = mem::take(&mut self.collection); - self.collection = Self::merge(collection, new_artist); - - Ok(()) } - pub fn delete_artist>(&mut self, artist_id: ID) -> Result<(), Error> { + pub fn remove_artist>(&mut self, artist_id: ID) { let index_opt = self .collection .iter() .position(|a| &a.id == artist_id.as_ref()); - match index_opt { - Some(index) => { - self.collection.remove(index); - Ok(()) - } - None => Err(Error::CollectionError(format!( - "artist '{}' is not in the collection", - artist_id.as_ref() - ))), + if let Some(index) = index_opt { + self.collection.remove(index); } } @@ -953,13 +940,14 @@ impl MusicHoard { artists } + fn get_artist(&mut self, artist_id: &ArtistId) -> Option<&mut Artist> { + self.collection.iter_mut().find(|a| &a.id == artist_id) + } + fn get_artist_or_err(&mut self, artist_id: &ArtistId) -> Result<&mut Artist, Error> { - self.collection - .iter_mut() - .find(|a| &a.id == artist_id) - .ok_or_else(|| { - Error::CollectionError(format!("artist '{}' is not in the collection", artist_id)) - }) + self.get_artist(artist_id).ok_or_else(|| { + Error::CollectionError(format!("artist '{}' is not in the collection", artist_id)) + }) } } @@ -1088,23 +1076,35 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + let mut expected: Vec = vec![]; - let actual_err = music_hoard.new_artist(artist_id.clone()).unwrap_err(); - let expected_err = Error::CollectionError(String::from( - "artist 'an artist' is already in the collection", - )); + music_hoard.add_artist(artist_id.clone()); + _ = expected.push(Artist::new(artist_id.clone())); + assert_eq!(music_hoard.collection, expected); + + music_hoard.add_artist(artist_id.clone()); + assert_eq!(music_hoard.collection, expected); + + music_hoard.remove_artist(&artist_id_2); + assert_eq!(music_hoard.collection, expected); + + music_hoard.remove_artist(&artist_id); + _ = expected.pop(); + assert_eq!(music_hoard.collection, expected); + } + + #[test] + fn collection_error() { + let artist_id = ArtistId::new("an artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + let actual_err = music_hoard + .add_musicbrainz_url(&artist_id, QOBUZ) + .unwrap_err(); + let expected_err = + Error::CollectionError(String::from("artist 'an artist' is not in the collection")); assert_eq!(actual_err, expected_err); assert_eq!(actual_err.to_string(), expected_err.to_string()); - - let actual_err = music_hoard.delete_artist(&artist_id_2).unwrap_err(); - let expected_err = Error::CollectionError(String::from( - "artist 'another artist' is not in the collection", - )); - assert_eq!(actual_err, expected_err); - assert_eq!(actual_err.to_string(), expected_err.to_string()); - - assert!(music_hoard.delete_artist(&artist_id).is_ok()); } #[test] @@ -1113,7 +1113,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); @@ -1184,7 +1184,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); @@ -1239,7 +1239,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); @@ -1369,7 +1369,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); @@ -1432,7 +1432,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); @@ -1562,7 +1562,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); @@ -1625,7 +1625,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.qobuz, expected); @@ -1676,7 +1676,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); + music_hoard.add_artist(artist_id.clone()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.qobuz, expected);