Minimize copy for MqttString and MqttBinary. #54
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
=======================================
Coverage 90.89% 90.90%
=======================================
Files 55 55
Lines 7460 7468 +8
=======================================
+ Hits 6781 6789 +8
Misses 679 679
🚀 New features to boost your workflow:
|
src/mqtt/packet/mqtt_string.rs
Outdated
| fn into_mqtt_string(self) -> Result<MqttString, MqttError> { | ||
| MqttString::new(self) | ||
| } | ||
| } |
There was a problem hiding this comment.
If you don't mind me asking, why create a custom trait instead of using TryInto? It looks like there are already some TryInto implementations, so this using it would duplicating them. And there's already a blanket implementation for types TryInto themselves. I implemented it here: smaeul@800b960
There was a problem hiding this comment.
I took another look. Using TryInto is indeed the better solution, as you mentioned. I've force-pushed an update to the PR. Could you take a look at the updated version?
src/mqtt/packet/mqtt_string.rs
Outdated
| type Error = MqttError; | ||
|
|
||
| fn try_from(s: &String) -> Result<Self, Self::Error> { | ||
| MqttString::new(s.as_str()) |
There was a problem hiding this comment.
I see the MqttBinary implementations use Self::new, whereas these use the concrete type, but this is a very minor inconsistency.
No description provided.