edgedb / edgedb-rust

The official Rust binding for EdgeDB
https://edgedb.com
Apache License 2.0
214 stars 26 forks source link

Expose bigint & decimal fields in public API #344

Closed aljazerzen closed 1 month ago

aljazerzen commented 1 month ago

Closes #271

CodesInChaos commented 1 month ago

I'm generally not a fan of exposing wire protocol details like that.

I'd at least put base10000 in the names of those properties, since most people would expect base 10 from those names. I'd also turn digits into a function that receives an index and returns an integer, so the internal representation can change, while preserving the API.

And even when working int base10000, I find that API quite awkward. If I'd design a base10000 API I'd use something like this:

// returns (bigint / 10000**position) mod 10000
// positions outside the range simply return 0
fn base10000_digit(&self, position: isize) -> u16.
// returns the range of digits which are represented (possibly non-zero) 
fn base10000_filled_digit_range(&self): Range<isize>;
bool is_negative(&self);

Where positions are isize for decimals, and usize for integers.

aljazerzen commented 1 month ago

~Yeah, your API does seem to make more sense. We can have both.~