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

byte buffer refactor #5486

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

12xx12
Copy link
Member

@12xx12 12xx12 commented Apr 7, 2023

  • Using smart pointer for allocation
  • Adding constants to the decoding for protocol values
  • minor copying and pasting stuff

I tried to maintain readability. I thought about using sizeof() to define the read/write byte count, but this seems a bit overkill and may lower readability.

- more constants
- more comments
- using smart pointer for memory
@12xx12 12xx12 changed the title C++update/byte buffer byte buffer refactor Apr 7, 2023
int Shift = 0;
unsigned char b = 0;
std::size_t Shift = 0;
unsigned char ReadByte = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer the simple b here, since English has the unfortunate ambiguity in Read (past tense vs imperative). ReadByte sounds like doReadByte to me, rather than theByteThatWasRead

@12xx12 12xx12 added this to the C++11 conversion milestone Sep 27, 2023
@12xx12 12xx12 enabled auto-merge (squash) September 27, 2023 19:49
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

Successfully merging this pull request may close these issues.

None yet

2 participants