Skip to content

Conversation

alexmojaki
Copy link

This has a few simple changes which I think improve the PPM code. The commits are mostly independent so if you like some changes but not others then it should be easy to merge a subset. Reading the commit diffs in isolation may also make them easier to understand. Happy to clarify the reasoning behind any particular change.

@nayuki
Copy link
Owner

nayuki commented Aug 8, 2022

I gave a second reading to each of your 5 commits and understand the intent of each one. They're not wrong, but they don't fit with my vision for the project. For example, there's no reason why the escape symbol couldn't be 0, and lazily allocating memory for subcontexts is a stylistic choice. I decided to close this as the outcome of the review.

@nayuki nayuki closed this Aug 8, 2022
@alexmojaki
Copy link
Author

there's no reason why the escape symbol couldn't be 0

  1. One obvious reason is that this prevents encoding 0 in the normal sense, it could easily appear in the data
  2. PpmCompress and PpmDecompress use hardcoded 256, so they actually don't support any other option
  3. One of those cases is if (symbol < 256), clearly replacing 256 with 0 would break the code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants