Filling a container with objects using pointers / iterators

I have a series of nested objects that I need to serialize into a byte (unsigned char) array to send out to another computer over UDP.

Right now I each object create a std::list, and pass that list to the next object up the 'tree' and then it merges the lists to create the final packet.

class Item {
  unsigned char value;
}

class Sublist {
  std::vector items;
  std::list subpacket;

  std::list Sublist::serialize() {
    subpacket.clear();
    for(auto item : items) {
      subpacket.push_back(item->value);
    }
    return subpacket;
  }
}

class Superlist {
  std::queue sublists;
  std::list serializedSublist;
  std::list packet;

  std::list Superlist::serialize() {
    packet.clear();
    for(auto sublist : sublists) {
      serializedSublist = sublist.serialize();
      packet.insert(packet.end(), serializedSublist.begin(), serializedSublist.end());
    }
    return packet;
  }

}

However, this feels really inefficient with unnecessary clear() and insert() calls, and in generally feels a bit sloppy.

I feel like a more efficient approach would be for the Superlist to have a unsigned char container (vector, list, deque?), and then give each Sublist a pointer to where it should write data to.

I feel like perhaps something like:

class Sublist {
  std::vector items;

  void Sublist::serialize(std::vector::iterator begin) {
    for(auto item : items) {
      begin->push_back(item->value);
      ++begin;
    }
  }
}

class Superlist {
  std::queue sublists;
  std::vector packet;

  std::list Superlist::serialize() {
    packet.clear();
    for(auto sublist : sublists) {
      sublist.serialize(packet.end());
    }
    return packet;
  }

}

Problems with this exact code include:

-
I don't think I can call push_back on a iterator

-
It'd be nice if I could create the final packet of a final size first, and then send pointers off to the correct location in the packet for each sublist to begin writing.

Would an approach like thi

std::list is usually implemented as a doubly-linked list which is very inefficient for your purpose. You should definitely use std::vector for this (or similar dynamic array structure, possibly some stream).

Your second approach is the idea I got while reading your question. You don't need an iterator (and I don't think you can call push_back on it), but a reference to the vector - vector& buffer as the argument.

The same applies to serizalize() on the main list with possible default argument creating the vector. If you want to have the final packet somewhere, you can track some dirty flag (if you can modify the structure) to re-serialize it as needed.

You could do that last approach in two phases:

• Track the sizes to compute the total size needed

• Allocate the buffer (you could use byte* or vector)

• Do the same again, but passing index together with the vector (buf[i++] = data)

...it is a bit more low-level solution, but possibly fastest. I would personally not worry that much and use the direct approach with vector (it should be fast enought).

Some notes about errors in the code:

class Sublist {
  std::vector items;
  std::list subpacket;

  std::list Sublist::serialize() {

You do use Sublist:: prefix when you define the methoud outside of the class, not here (remove it).

class Sublist { ...
    std::vector& serialize(std::vector& buffer) const { ...

This should be the final signature (taking reference, returning the reference and with const modifier on the method because you are not modifying the class).

StackExchange Code Review Q#65222, answer score: 4