HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Binary serialization library

Submitted by: @import:stackexchange-codereview··
0
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 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:

  • 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.