# Simple packet parsing command pattern

I'm currently creating a small server back-end. My goal is to write an (efficient) packet parser in C++17 using the command pattern. The packets are given by:

type: value in {0, 1, 255}
size: value in {0, 1, 65535}
data: array[size]


Each packet is processed by a packet_operator. The packet_registry is a packet_operator which simply calls the operator for the given packet type. I currently see no need for multiple, pipelined parsers for a single packet type. The code sample is given below and does compile without warnings.

packets.hpp

#pragma once

#include <array>
#include <memory>

// Test class without proper implementation.
class packet {

public:
uint32_t m_type = 0;

uint32_t type() const {
return m_type;
}

};

template<class ... ArgTypes>
class packet_operator {

public:
virtual void operator()(const packet &, ArgTypes... args) = 0;

};

template<class ... ArgTypes>
class packet_registry : public packet_operator<ArgTypes...> {

public:
using op_ptr = std::shared_ptr<packet_operator<ArgTypes...>>;

explicit packet_registry(op_ptr op) {
// Initialize registry with default processor
for (size_t i = 0; i < m_operators.size(); i++)
m_operators[i] = op;
}

void operator()(const packet &packet, ArgTypes... args) override {
// Call processor for packet type
(*m_operators[packet.type()])(packet, args...);
}

op_ptr &operator[](uint32_t i) {
return m_operators[i];
}

private:
std::array<op_ptr, 256> m_operators;  // Type will always be in [0, 255]

};


main.cpp

#include <iostream>
#include <memory>

#include "packets.hpp"

class processor : public packet_operator<> {

int m_id;

public:
explicit processor(int id) : m_id(id) {
// Empty
}

void operator()(const packet &packet) override {
std::cout << "Processor " << m_id << ". Type: " << packet.type() << std::endl;
}

};

void init(packet_registry<> &registry) {
registry[1] = std::shared_ptr<packet_operator<>>(new processor(1));
registry[2] = std::shared_ptr<packet_operator<>>(new processor(2));
}

int main() {
// Create registry with default processor
auto registry = packet_registry<>(
std::shared_ptr<packet_operator<>>(new processor(0)));

// Initialize packet processors
init(registry);

// Test packets with type 0, 1, 2 and 3
packet p0, p1, p2, p3;
p0.m_type = 0;
p1.m_type = 1;
p2.m_type = 2;
p3.m_type = 3;

registry(p0);
registry(p1);
registry(p2);
registry(p3);

return 0;
}


My main concern is my use of functors and shared pointers. Did I use those correctly? Additionally, my linter advised me to add the explicit keyword before the constructors. I've researched what it does, but do not really understand why those types of constructors would always need to be explicit.

I'd also like to know if there are any alternatives to my init method. I'd have to register the processors for every packet type here. I'd prefer some sort of self-registering processor. For example, in Java I could annotate all packet processors using a custom annotation. Those will then be added to the right registry using reflection. Maybe this is achievable through some sort of meta-programming?

Lastly, a more more general question: are there any alternative designs, given that I cannot change the protocol?

• Not worthy as an answer, but capitalizing classes makes it easier to read – David Jun 2 '18 at 4:55
• @David That is worthy as an answer. Reviewing code in comments is considered bad form here. – Daniel Jun 2 '18 at 9:18
• @David I chose to stick with the already existing std/boost style and not impose some arbitrary style (e.g. Google) onto my project. – Rev Jun 2 '18 at 11:33
• @Rev I wouldn't say arbitrary, but it is personal preference so it's more a suggestion. Having class names capitalized seems to make code easier to read (processor processor vs Processor processor), but it may depend on your background. – David Jun 2 '18 at 14:11
• @David it actually plays quite well in large projects to name framework-y classes underscore separated lowercase and application classes camel case – bobah Jul 4 '18 at 6:22

The keyword explicit should be used with constructors taking a single argument in classes when automatic instantiation is not a universally valid operation (details and examples here).
• The packet_registry can just be an array of std::function (as you seem to be OK with paying for the virtual dispatch overhead) — std::array<std::function< packet const&, ArgTypes...>, 256>
• processor-s, given they are created just once, are better be created by value. Then you will be able to have a very hackable syntax — registry[x] = [...](auto&& packet, auto&& arg) { ... actual code ...};
• if for some reason you do need shared pointers, prefer make_shared over direct usage of the new operator — x = std::make_shared<type>(args...).