Draft: TxtProperties: new method to get a HashMap #303
Conversation
src/service_info.rs
Outdated
| @@ -499,6 +499,15 @@ impl TxtProperties { | |||
| pub fn get_property_val_str(&self, key: &str) -> Option<&str> { | |||
| self.get(key).map(|x| x.val_str()) | |||
| } | |||
|
|
|||
| /// Returns a map of properties, where the key is the property key. | |||
| pub fn get_property_map_str(&self) -> HashMap<String, String> { | |||
There was a problem hiding this comment.
Can we consume the TxtProperties? Something like:
/// Returns a map of properties, where the key is the property key.
///
/// **This method consumes [`TxtProperties`].**
pub fn into_hashmap(self) -> HashMap<String, String>One can always obtain an HasMap<&str, &str> using the current methods provided by TxtProperties
There was a problem hiding this comment.
There are 2 commits. The 2nd commit already consumes the TxtProperties.
src/service_info.rs
Outdated
| /// Returns a map of properties, where the key is the property key. | ||
| pub fn get_property_map_str(&self) -> HashMap<String, String> { | ||
| let mut map = HashMap::new(); | ||
| for prop in self.properties.iter() { |
There was a problem hiding this comment.
If we consider the comment above, I would use into_iter() here
There was a problem hiding this comment.
same here, it's in the 2nd commit ;-)
src/service_info.rs
Outdated
| pub fn get_property_map_str(&self) -> HashMap<String, String> { | ||
| let mut map = HashMap::new(); | ||
| for prop in self.properties.iter() { | ||
| map.insert(prop.key.clone(), prop.val_str().to_string()); |
There was a problem hiding this comment.
same here, it's in the 2nd commit.
src/service_info.rs
Outdated
| ( | ||
| p.key, | ||
| p.val | ||
| .map_or_else(String::new, |v| String::from_utf8(v).unwrap_or_default()), |
There was a problem hiding this comment.
I would filter out empty String or wrong ones. What do you think? In this way the HashMap will have only the correct ones.
There was a problem hiding this comment.
Sounds good! Will update.
| self.properties | ||
| .into_iter() | ||
| .filter_map(|p| { | ||
| // Skip the property if the value is non-empty and not valid UTF-8. |
There was a problem hiding this comment.
The comment seems wrong. We should skip if the value is empty and not valid UTF-8
| // Skip the property if the value is non-empty and not valid UTF-8. | ||
| let value = p | ||
| .val | ||
| .map_or(Some(String::new()), |v| String::from_utf8(v).ok())?; |
There was a problem hiding this comment.
I would use is_some_and instead of map_or, in this way we do not consider empty Strings
This is to follow up on issue #300 .