Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
iostat opened this issue Dec 10, 2022 · 1 comment
Open

Comments

@iostat
Copy link

iostat commented Dec 10, 2022

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants