search

andylokandy / byte

A low-level, zero-copy, panic-free, binary serializer and deserializer. (parser and encoder)
MIT License
42 stars 7 forks source link

TryRead and TryWrite for u8 and i8 should be agnostic to context. #9

Open iostat opened 1 year ago

iostat commented 1 year ago

Currently, in the case of u8 and i8, only TryRead<Endian> and TryWrite<Endian> are implemented. In reality, endianness doesn't matter for single byte values, and you end up writing verbose code that seems to hint that there's some sort of different handling of the bytes. 

// these two are the same net result
let u1: u8 = bytes.read_with(ofs, byte::ctx::Endian::Little)?;
let u2: u8 = bytes.read_with(ofs, byte::ctx::Endian::Big)?;

// ditto
let i1: i8 = bytes.read_with(ofs, byte::ctx::Endian::Little)?;
let i2: i8 = bytes.read_with(ofs, byte::ctx::Endian::Big)?;

// yet i cant use the default context (`()`) even though it makes my code a lot more readable 
// and doesn't leave me wondering why I need to specify an endianness for a single-byte value.
let sensible: u8 = bytes.read(ofs);
let sensible2: i8 = bytes.read(ofs);

Perhaps the TryRead for u8 and i8 should really be impl<'a, Ctx> TryRead<'a, Ctx>? Or at the very least, also have a TryRead<()>/TryWrite<()> like bool (and conversely, maybe bool should be generic on context)?

andylokandy commented 1 year ago

Hey, thanks for the suggestion. I really appreciate your input. My advice would be to implement i8 and u8 with the () context. If you're interested, I'd love it if you could submit a pull request with your proposed changes.