Looks well written overall. Good job. I'll just give a few quick remarks about the code. Compression is not really my field. - Unnecessary destructors in `bofstream` and `bifstream`. They already hold Standard streams that automatically close the underlaying file handles in their own destructors, so you're just replicating work by calling `close()` explicitly. Once you remove that, you'll end up with empty destructors that should be removed. Let the compiler supply a default when you have no manual cleanup to perform in your classes. Actually, `bofstream` might need to `flushBufferToFile()`, so that one might need a destructor after all... - Mark methods that aren't changing member data with `const`. E.g.: `eof`, `good`, `fail`, etc. Take a look into [what const member functions mean in C++][1]. This is an important thing to use and I see that you are not being consistent with it. - `bofstream::put(std::vector<bool>)` is taking the vector parameter by value, thus making its own copy of it just to iterate the array. You don't need that copy if you're just inspecting the collection. Pass by *const reference* instead: void put(const std::vector<bool> & v) { ... } ^^^^^ ^ In general you should pass objects by reference if you're only looking at the object without wanting to store an actual copy of it somewhere. Native types like integers are of course still passed by value, since copying those is free and implemented in the hardware. - Use *range based for loops* when you're iterating a standard collection from back-to-back. Instead of this: > for (auto i = bitset.begin(); i != bitset.end(); i++) You can do this: for (auto b : bitset) { // use b } - Prefixing member variables or methods with `this->` is generally not a good practice. Not only it is unnecessary and verbose, but it can also hide names that shadow each other, which can be a serious problem. - `Byte`'s default constructor doesn't initialize its member data. *Always* initialize data to some known value to make your code deterministic. - `Byte`'s destructor is empty, so should be removed. Same as mentioned in the fist point. - Why didn't you initialize all members of `CharCountNode` in the initializer list? > CharCountNode::CharCountNode(Byte b, int i) : byte(b.getChar(), b.getIsTerminator()) > { > count = i; > left = nullptr; > right = nullptr; > } - Use [`std::snprintf`][2] over `sprintf_s` or `sprintf`. It takes a count parameter to prevent buffer overruns. [1]: https://isocpp.org/wiki/faq/const-correctness#const-member-fns [2]: http://en.cppreference.com/w/cpp/io/c/fprintf