Remove special case handling of empty string values

CHANGELOG.md

@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- **APE**/**ID3v1**/**ID3v2**/**Tag**
+  - Allow empty strings as values instead of removing the corresponding item when empty ([PR](https://github.com/Serial-ATA/lofty-rs/pull/134))
+
 ## [0.11.0] - 2022-1-29
 
 ### Added

src/ape/tag/mod.rs

@@ -33,11 +33,6 @@ macro_rules! impl_accessor {
 				}
 
 				fn [<set_ $name>](&mut self, value: String) {
-					if value.is_empty() {
-						self.[<remove_ $name>]();
-						return;
-					}
-
 					self.insert(ApeItem {
 						read_only: false,
 						key: String::from(crate::tag::item::first_key!($($key)|*)),

src/id3/v1/tag.rs

@@ -24,11 +24,6 @@ macro_rules! impl_accessor {
 				}
 
 				fn [<set_ $name>](&mut self, value: String) {
-					if value.is_empty() {
-						self.[<remove_ $name>]();
-						return;
-					}
-
 					self.$name = Some(value)
 				}
 

src/id3/v2/tag.rs

@@ -34,11 +34,6 @@ macro_rules! impl_accessor {
 				}
 
 				fn [<set_ $name>](&mut self, value: String) {
-					if value.is_empty() {
-						self.remove($id);
-						return;
-					}
-
 					self.insert(Frame {
 						id: FrameID::Valid(String::from($id).into()),
 						value: FrameValue::Text {
@@ -694,28 +689,30 @@ impl SplitAndMergeTag for ID3v2Tag {
 	}
 
 	fn merge_tag(&mut self, mut tag: Tag) {
-		fn join_items(tag: &mut Tag, key: &ItemKey) -> String {
+		fn join_items(tag: &mut Tag, key: &ItemKey) -> Option<String> {
 			let mut iter = tag.take_strings(key);
-
-			match iter.next() {
-				None => String::new(),
-				Some(first) => {
-					let mut s = String::with_capacity(iter.size_hint().0);
-					s.push_str(&first);
-					iter.for_each(|i| {
-						s.push(V4_MULTI_VALUE_SEPARATOR);
-						s.push_str(&i);
-					});
-
-					s
-				},
-			}
+			iter.next().map(|first| {
+				// Use the length of the first string for estimating the capacity
+				// of the concatenated string.
+				let estimated_len_per_item = first.len();
+				let min_remaining_items = iter.size_hint().0;
+				let mut concatenated = first;
+				concatenated.reserve((1 + estimated_len_per_item) * min_remaining_items);
+				iter.for_each(|i| {
+					concatenated.push(V4_MULTI_VALUE_SEPARATOR);
+					concatenated.push_str(&i);
+				});
+				concatenated
+			})
 		}
 
 		self.frames.reserve(tag.item_count() as usize);
 
-		let artists = join_items(&mut tag, &ItemKey::TrackArtist);
-		self.set_artist(artists);
+		if let Some(artists) = join_items(&mut tag, &ItemKey::TrackArtist) {
+			self.set_artist(artists);
+		} else {
+			self.remove_artist()
+		}
 
 		for item in tag.items {
 			let frame: Frame<'_> = match item.into() {

src/tag/mod.rs

@@ -27,11 +27,6 @@ macro_rules! impl_accessor {
 				}
 
 				fn [<set_ $name>](&mut self, value: String) {
-					if value.is_empty() {
-						self.[<remove_ $name>]();
-						return;
-					}
-
 					self.insert_item(TagItem::new(ItemKey::$item_key, ItemValue::Text(value)));
 				}
 
@@ -719,13 +714,16 @@ mod tests {
 	}
 
 	#[test]
-	fn insert_empty() {
+	fn should_preserve_empty_title() {
 		let mut tag = Tag::new(TagType::ID3v2);
 		tag.set_title(String::from("Foo title"));
 
 		assert_eq!(tag.title().as_deref(), Some("Foo title"));
 
 		tag.set_title(String::new());
+		assert_eq!(tag.title().as_deref(), Some(""));
+
+		tag.remove_title();
 		assert_eq!(tag.title(), None);
 	}
 }