aliri_braid icon indicating copy to clipboard operation
aliri_braid copied to clipboard

Implement `Borrow<String>`?

Open guilload opened this issue 2 years ago • 3 comments

Hello, Would you consider adding impl Borrow<String> for the owned type?

guilload avatar Apr 13 '24 12:04 guilload

I think that I would need to better understand the use case for implementing Borrow<String>. In the vast majority of cases, the Borrow<RefType> and Borrow<str> implementations cover the use cases of borrowing the value for searching in a HashMap or BTreeMap. In general, Borrow<String> and AsRef<String> don't get you much more than you already get with Borrow<str> or AsRef<str>, as you'd only get an immutable reference to the underlying buffer, so could not do any mutation (and with a &String, you pay for a double pointer indirection to access the underlying value).

I'll note that, due to the nature of this code generation, adding Borrow<String> would be considered a breaking change, as if there is anyone out there who manually added a Borrow<String> impl, then there would be two conflicting impls.

neoeinstein avatar Apr 16 '24 17:04 neoeinstein

Thanks for your answer. I'm working with some structs imported from some external crates with some String fields. I want to reduce the verbosity of my code when performing some hashmap lookups as follows:

use some_crate::SomeType. // SomeType { some_field: String };

#[braid]
struct MyType(String)

let my_objects: HashMap<MyType, T> = ...


// Without `impl Borrow<String> for MyType`
let my_field_ref = MyTypeRef::from_str(&some_type.some_field);
let my_object_opt = my_objects.get(my_field_ref);

// With `impl Borrow<String> for MyType`
let my_object_opt = my_objects.get(&some_type.some_field);

That would be nice to have, but I can always add the impl Borrow<String> manually for the dozen types I have.

guilload avatar Apr 16 '24 18:04 guilload

Thanks for preparing the use case for me. I think that, in these situations, I generally defer to .as_str(), as in:

let my_object_opt = my_objects.get(some_type.some_field.as_str());
// or
let my_object_opt = my_objects.get(&*some_type.some_field);

Certainly less typing would be preferable, so I can get the appeal of Borrow<String> for this. Thanks for the suggestion.

neoeinstein avatar Apr 16 '24 20:04 neoeinstein