Encode via `Display` adapter as an alternative to going through `String`

[abonander]

In our projects at work, wherever we're using hash IDs, I like to create adaptor structs to make them easier to work with:

static CODEC: std::sync::Lazy<harsh::Harsh> = Lazy::new(|| /* init `Harsh` */);

pub struct SomeHashedId(pub u64);

impl FromStr for SomeHashedId {
    // implement using `Harsh::decode()`
}

impl Display for SomeHashedId {
    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
        f.pad(&CODEC.encode(&[self.0]))
    }
}

However, implementing Display using Harsh::encode() is inefficient since it requires going through String. I would really like to see something like:

impl Harsh {
    // it'd then be trivial to implement `encode` in terms of this method
    pub fn encode_fmt<'a>(&'a self, ids: &'a [u64]) -> impl Display + 'a {
        // ...
    }
}

so the above Display impl could be implemented as:

impl Display for SomeHashedId {
    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
        CODEC.encode_fmt(&[self.0]).fmt(f)
    }
}

[abonander]

Looking at the implementation of encode(), it seems to depend on having random access to the String it keeps as a buffer so there may not really be anything to gain reimplementing it as a Display adapter.

Opinions?

[archer884]

Hey, I just noticed this. I told my phone to remind me to review this after my workshop this afternoon. Just leaving this note now so you don't feel like I'm totally ignoring you. :)

[archer884]

@abonander, yeah, this library is translated pretty much directly from JavaScript, and JavaScript doesn't offer nice toys like this, so the Rust translation doesn't, either. :(

I'm not saying it's impossible. It looks like part of what that encode segment does is ensure that the correct minimum length is maintained for the output. For that reason, the first thing you'd need is a way to know, before actually formatting anything, how long a given hashid is going to be. There is, however, a limit to how much more efficient this can become, because one thing done on both encoding and decoding is mutation of the alphabet, which is then used to create padding to meet the min length requirement, which means the encoding steps leading up to that point can't really be ignored.

Note: It's also possible, because the encoding offers these marker/stop bytes or whatever, that the padding is entirely ignored and won't result in a failure to decode if it is wrong---but failing to shuffle the alphabet used for the padding will result in all the padding looking the same. The encoding process is just inherently stateful.

On the up side, the project already has some pretty decent tests and a benchmark harness if you want to play with figuring out some way to get that implemented. It would be pretty easy to rig up a benchmark to determine whether you actually gain anything or not.