patterncppMinor
Binary serialization library
Viewed 0 times
serializationbinarylibrary
Problem
I am currently working on a binary serialization library written in C++11 for a personal project. I'd really like to have a review about my design, my implementation and everything else. The library is inspired from
Here is the GitLab repository containing the project.
Sery/Misc.hh
Sery/Buffer.hh
Sery/Buffer.cpp
```
#include
#include
#include
#include
SERY_BEGIN_NAMESPACE
Buffer::Buffer()
: _buffer()
{
}
Buffer::Buffer(const char *buffer, uint32 size)
: _buffer(buffer, buffer + size)
{
}
Buffer::~Buffer()
{
}
void Buffer::writeRaw(const char *buffer, uint32 size)
{
_buffer.insert(_buffer.end(), buffer, buffer + size)
QDataStream.Here is the GitLab repository containing the project.
Sery/Misc.hh
#ifndef SERY_MISC_HH_
#define SERY_MISC_HH_
#define SERY_BEGIN_NAMESPACE namespace Sery {
#define SERY_END_NAMESPACE }
#include
#include
SERY_BEGIN_NAMESPACE
template
using enable_if_t = typename ::std::enable_if::type;
enum Endian
{
LittleEndian,
BigEndian
};
typedef std::int8_t int8;
typedef std::int16_t int16;
typedef std::int32_t int32;
typedef std::int64_t int64;
typedef std::uint8_t uint8;
typedef std::uint16_t uint16;
typedef std::uint32_t uint32;
typedef std::uint64_t uint64;
namespace detail
{
inline Endian getSoftwareEndian()
{
int16 witness = 0x5501;
int8 test = *((int8*)&witness);
return (test == 1 ? Endian::LittleEndian : Endian::BigEndian);
}
}
SERY_END_NAMESPACE
#endif // SERY_MISC_HH_Sery/Buffer.hh
#ifndef SERY_BUFFER_HH_
#define SERY_BUFFER_HH_
#include
#include
SERY_BEGIN_NAMESPACE
class Buffer : public IBuffer
{
public:
Buffer();
Buffer(const char *buffer, uint32 size);
~Buffer();
public:
virtual void writeRaw(const char *buffer, uint32 size) final;
virtual void readRaw(char *buffer, uint32 size) final;
virtual uint32 size() const final;
virtual const char *data() const final;
private:
std::vector _buffer;
};
SERY_END_NAMESPACE
#endif // SERY_BUFFER_HH_Sery/Buffer.cpp
```
#include
#include
#include
#include
SERY_BEGIN_NAMESPACE
Buffer::Buffer()
: _buffer()
{
}
Buffer::Buffer(const char *buffer, uint32 size)
: _buffer(buffer, buffer + size)
{
}
Buffer::~Buffer()
{
}
void Buffer::writeRaw(const char *buffer, uint32 size)
{
_buffer.insert(_buffer.end(), buffer, buffer + size)
Solution
A few notes:
-
your setter and getter for GlobalEndian only set and get the value;
Better solution: consider setting the value itself public and removing the accessors (they don't add anything and effectively they expose the value as if it were public).
Even better solution: Ensure that each class has it's own (private) copy of the endianness flag and initialize the private copy upon construction (with a default value); Instances should have no public getter or setter. This is because you cannot change the endianness in the middle of writing to the stream and still expect the written data to be valid.
The way you arrange the code by columns makes it (marginally) easier to read, but over the lifetime of a project, you will either get some tokens out of sync with each other, or have to realign entire lists of functions when you change the length of an identifier; it is debatable if the extra ease in reading the code is worth it - because once you get used to reading untabulated code, the extra alignment adds nothing but extra maintenance effort).
- stop using C-style casts; Better alternative exists
- stop wraping return expressions in parenthesis (every time I see that I think "this is a C-style value cast ... no wait!")
- do not generate code using macros; the namespace macros should only be used if you are going to build on a platform that has no namespace support (are you? you didn't specify)
-
your setter and getter for GlobalEndian only set and get the value;
Better solution: consider setting the value itself public and removing the accessors (they don't add anything and effectively they expose the value as if it were public).
Even better solution: Ensure that each class has it's own (private) copy of the endianness flag and initialize the private copy upon construction (with a default value); Instances should have no public getter or setter. This is because you cannot change the endianness in the middle of writing to the stream and still expect the written data to be valid.
The way you arrange the code by columns makes it (marginally) easier to read, but over the lifetime of a project, you will either get some tokens out of sync with each other, or have to realign entire lists of functions when you change the length of an identifier; it is debatable if the extra ease in reading the code is worth it - because once you get used to reading untabulated code, the extra alignment adds nothing but extra maintenance effort).
Context
StackExchange Code Review Q#98395, answer score: 4
Revisions (0)
No revisions yet.