Changing Original Library API when writing Wrappers

[Ayush Singh]

Ok, so for a more involved example, take the case of a method in KConfigGroup:
`void writePathEntry (const QString &pKey, const QString &path,
WriteConfigFlags pFlags=Normal)`

Here, the path has a type of QString. In Rust, we can instead use the
type `Path` to represent this and convert it to `QString` using
`TryFrom` trait before passing to C++.

This change would mean that first a `PathBuf` or `Path` is constructed
and then is converted to `QString`. Since this conversion can fail, we
will need to return a `Result`. The function definition for this new
function will be something like this:
```
pub fn write_path_entry(&mut self, pkey: QString, path: &Path, pflags:
WriteConfigFlags) -> Result<(), Error> {
    let path = QString::try_from(path)?;
    .....
    Ok(())
}
```

Here, the API changes significantly. If the path is being allocated at
runtime, it incurs the cost of constructing `PathBuf` on the heap
along with the normal cost of `QString`. However, this path is now
significantly more useful if we have to interact with Rust libraries.
Also, it automatically checks that the path actually looks like a Path
instead of just any random String. Since we are passing a reference,
we do not have to clone the `Path`, unlike `QString`, so I don't think
the cost should be much more than just QString for multiple uses.

So what do you suggest in this case? I think most medium to big
projects would use `Path` regardless but small projects might end up
using QString. It is also an option to keep both forms with different
function names.

Ayush Singh

On Mon, Jan 31, 2022 at 4:13 AM David Hurka <david.hurka at mailbox.org> wrote:
>
> On Sunday, January 30, 2022 2:20:10 PM CET Ayush Singh wrote:
> > An example of this situation:
> > `QString readGenericName () const`
> > method of KDesktopFile
> > (https://api.kde.org/frameworks/kconfig/html/classKDesktopFile.html#aaf263b7
> > 9cce3125c9e6e52428e05c524). If I am doing a one-to-one wrapping, the Rust
> > function definition
> > would look something like this:
> > `fn read_generic_name(&self) -> QString`
> > By default, this function returns `QString()` in case the key is not
> > present in the `.desktop` file. According to me, a better definition
> > for this function will be:
> > `fn read_generic_name(&self) -> Option<QString>`
> > The function will return `None` if the key is not present in this case.
>
> I remember that there were plans to use std::optional<> for return types in a
> future version, so KConfig would end up to use the C++ interface you propose
> for Rust. So your suggestion seems appropriate in this special case.
>
> Besides that, I think that this example is fairly low-level. If you adapt
> towards Rust here, you preserve the overall way to use the library, and you
> just have a more elegant way to reach your goal.
>
> Cheers, David
>
>