Data structure to store large numbers of runtime-dependent arithmetic types

Question

Warning: lots of code. If it's too much, please focus primarly on channel.h and channel.hpp.

This is my first time posting on Code Review, so I apologize in advance if anything is unusual or could have been written better.

Context

The program in which I am using this data structure is a DAQ to CSV conversion tool. Put simply, the goal is to read from binary and write to text. Ignoring a small header which describes its contents, the DAQ file is structured using "frames" and "channels". Each frame contains a number of channels. Each channel has a fixed number of items per channel. For example, I can have three channels: A, B, and C, where A contains 3 items per frame, B contains 1 item per frame, and C contains 20 items per frame.

There are three things to consider: first, the channels in each frame are not guaranteed to be consecutive. Using the example above, on frame 1, I might have data for channels A, B, and C, but on frame 2, I may only have data for channels A and C.

Second, each channel has its own arithmetic type, indicated in the header by a char. For example, channel A may contain data of type 'd' (double), B with type 'i' (integer), and C with type 'c' (char). This means that even the type of the items being read is not known until runtime.

Third, all this data must be read and stored within the program to allow the user to selectively choose what channels / data to print, as well as create their own channels using existing ones. This requires the method of storage to be very efficient in both space and speed. A typical DAQ file ranges from 300MB to 400MB, with +65,000 frames, +250 channels, and up to +600 items in a channel. For perspective, in one case, I wrote all items from a DAQ file to CSV. Excel reported more than 2.5 million cells.

Code

This is the data structure (class Channel) I designed for this task. I have included the relevant files and dependencies. I've also included a sample main.cpp to illustrate how channel is intended to be used.

The code uses templates to handle the variable arithmetic types, smart pointers and polymorphic structs to store the template struct, an internal type (Data::Type) in combination with static asserts and exceptions for type safety, and C++14 return type deduction and generic lambdas to access data. Please let me know what you think. Thank you very much!

main.cpp

#include "channel.h"

#include <iostream>
#include <vector>

int main() {
  // User-defined channels.
  auto c1 = Channel{Channel::create<int >("ints  :")};
  auto c2 = Channel{Channel::create<char>("chars :")};

  // Sample data.
  auto v1 = std::vector<int>{1, 2, 3};
  auto v2 = std::vector<char>{'a', 'b', 'c'};

  // Push-back method for user-defined channels.
  // User-defined channels can only have one item per frame.
  // SFINAE also provides clear error messages if a user
  // attempts to push back a type which does not match internals.
  for (auto i = std::size_t{}; i < v1.size(); ++i) c1.push_back(i, v1[i]);
  for (auto i = std::size_t{}; i < v2.size(); ++i) c2.push_back(i, v2[i]);

  // Push-back method for DAQ channels with multiple items.
  // During conversion, size of vector can range from 1 to +600.
  c1.push_back(4, v1);
  c2.push_back(4, v2);

  auto const print = [](auto const& data) {
    for (auto datum : data) std::cout << datum << ' ';
    std::cout << std::endl;
  };

  // Access data using generic lambdas.
  // This provides a type-safe way to access the templated
  // vector (or individual datum by frame and index) without
  // the user needing to know the underlying type (use casts).
  c1.data(print);
  c2.data(print);

  // Access data by frame and index (for channels with multiple items).
  // Channels should have a consistent number of items per frame, and
  // no duplicate frame numbers (preferably, in increasing order as well).
  c1.datum(2, 0, [](auto datum) { std::cout << datum << " == 3\n"; });
  c2.datum(4, 2, [](auto datum) { std::cout << datum << " == c\n"; });
}

record.h

#ifndef RECORD_H
#define RECORD_H

#include "data.h"

#include <string>

struct Record {
  int32_t     id;                           // ID in DAQ file.
  int32_t     items;                        // Number of items.
  std::string name = std::string(53, '\0'); // Name of channel.
  int16_t     rate;                         // Rate of updates.
  Data::Type  type;                         // Type of data.
  int32_t     varlen;                       // Variable length (?)
};

#endif

data.h

#ifndef DATA_H
#define DATA_H

#include <stdexcept>
#include <vector>

// Abstract base class for DataT<T>.
struct Data {
  // Used to store internal type information in class Channel.
  enum struct Type : char {
    Double = 'd'
  , Float  = 'f'
  , Int    = 'i'
  , Short  = 's'
  , Char   = 'c'
  };

  // Template Metaprogramming. Returns corresponding Data::Type for type T.
  template <typename T> static Type get_type() {
    static_assert(std::is_arithmetic<T>::value, "type T must be arithmetic");
    if (std::is_same<T, double>::value) return Data::Type::Double;
    if (std::is_same<T, float >::value) return Data::Type::Float;
    if (std::is_same<T, int   >::value) return Data::Type::Int;
    if (std::is_same<T, short >::value) return Data::Type::Short;
    if (std::is_same<T, char  >::value) return Data::Type::Char;
    throw std::logic_error("invalid type T in Data::get_type<T>()");
  }

  // Mark class as abstract.
  virtual ~Data() = 0;
};

// Destructor must still be defined.
inline Data::~Data() = default;

// Concrete derived class used to store data in class Channel.
template <typename T> struct DataT : Data { std::vector<T> data; };

#endif

channel.h

#ifndef CHANNEL_H
#define CHANNEL_H

#include "data.h"
#include "record.h"

#include <memory>
#include <vector>

class Channel {
  std::unique_ptr<Data> m_data;
  std::vector<int>      m_frames;
  Record                m_record;

  // Private Constructor.
  Channel(std::unique_ptr<Data> data, Record record)
  : m_data(std::move(data)), m_record(std::move(record)) { }

  // Private Data Interface Function.
  template <typename Function> auto get_data(Function func);

public:
  // Disable default constructors. Enable moves only.
  Channel()                       = delete;
  Channel(Channel const&)         = delete;
  auto operator=(Channel const&)  = delete;
  Channel(Channel&&)              = default;

  // Factory Method Pattern.
  template <typename T> static Channel create(std::string name);

  static Channel create(Record record) {
    switch (record.type) {
    case Data::Type::Double : return {std::make_unique<DataT<double>>(), std::move(record)};
    case Data::Type::Float  : return {std::make_unique<DataT<float >>(), std::move(record)};
    case Data::Type::Int    : return {std::make_unique<DataT<int   >>(), std::move(record)};
    case Data::Type::Short  : return {std::make_unique<DataT<short >>(), std::move(record)};
    case Data::Type::Char   : return {std::make_unique<DataT<char  >>(), std::move(record)};
    default : throw std::invalid_argument("invalid Data::Type in Channel::create(Data::Type)");
    }
  }

  // Channel Interface Functions.
  decltype(m_frames) const& frames() const { return m_frames;        }
  decltype(m_record) const& record() const { return m_record;        }
  std::size_t               size()   const { return m_frames.size(); }

  // Data Interface Functions.
  template <typename T> decltype(DataT<T>::data) const& data() const;
  template <typename T> T const& datum(int frame, int offset) const;
  template <typename Function> auto data(Function func) const;
  template <typename Function> auto datum(int frame, int offset, Function func) const;

  // Record Interface Functions.
  decltype(Record::id)     const& id()     const { return m_record.id;     }
  decltype(Record::items)  const& items()  const { return m_record.items;  }
  decltype(Record::name)   const& name()   const { return m_record.name;   }
  decltype(Record::rate)   const& rate()   const { return m_record.rate;   }
  decltype(Record::type)   const& type()   const { return m_record.type;   }
  decltype(Record::varlen) const& varlen() const { return m_record.varlen; }

  // Channel Initialization Functions. Uses SFINAE for descriptive error messages.
  template <typename T> auto push_back(T)                                     -> std::enable_if_t<!std::is_arithmetic<T>::value>;
  template <typename T> auto push_back(int frame, T datum)                    -> std::enable_if_t< std::is_arithmetic<T>::value>;
  template <typename T> auto push_back(int frame, std::vector<T> const& data) -> std::enable_if_t< std::is_arithmetic<T>::value>;

private:
  // Enforce type match between internal type and template type deduction.
  template <typename T> void check_type() const;
};

#include "channel.hpp"

#endif

channel.hpp

/** Included by channel.h. **/

template <typename Function> auto Channel::get_data(Function func) {
  switch (m_record.type) {
  case Data::Type::Double : return func(static_cast<DataT<double>*>(m_data.get())->data);
  case Data::Type::Float  : return func(static_cast<DataT<float >*>(m_data.get())->data);
  case Data::Type::Int    : return func(static_cast<DataT<int   >*>(m_data.get())->data);
  case Data::Type::Short  : return func(static_cast<DataT<short >*>(m_data.get())->data);
  case Data::Type::Char   : return func(static_cast<DataT<char  >*>(m_data.get())->data);
  default : throw std::logic_error("invalid Data::Type set in Channel");
  }
}

template <typename T> Channel Channel::create(std::string name) {
  static_assert(std::is_arithmetic<T>::value, "type T must be arithmetic.");
  auto record = Record{0, 1, std::move(name), 1, Data::get_type<T>(), 0};
  return {std::make_unique<DataT<T>>(), std::move(record)};
}

template <typename T> decltype(DataT<T>::data) const& Channel::data() const try {
  check_type<T>();
  return static_cast<DataT<T> const*>(m_data.get())->data;
} catch (std::invalid_argument const& e) {
  throw std::invalid_argument(std::string(e.what()).append(" in Channel::data<T>()"));
}

template <typename T> T const& Channel::datum(int frame, int offset) const try {
  check_type<T>();
  auto index = std::distance(m_frames.begin(), std::lower_bound(m_frames.begin(), m_frames.end(), frame));
  return static_cast<DataT<T> const*>(m_data.get())->data[index * m_record.items + offset];
} catch (std::invalid_argument const& e) {
  throw std::invalid_argument(std::string(e.what()).append(" in Channel::datum<T>(int, int)"));
}

template <typename Function> auto Channel::data(Function func) const {
  switch (m_record.type) {
  case Data::Type::Double : return func(static_cast<DataT<double> const*>(m_data.get())->data);
  case Data::Type::Float  : return func(static_cast<DataT<float > const*>(m_data.get())->data);
  case Data::Type::Int    : return func(static_cast<DataT<int   > const*>(m_data.get())->data);
  case Data::Type::Short  : return func(static_cast<DataT<short > const*>(m_data.get())->data);
  case Data::Type::Char   : return func(static_cast<DataT<char  > const*>(m_data.get())->data);
  default : throw std::logic_error("invalid Data::Type set in Channel");
  }
}

template <typename Function> auto Channel::datum(int frame, int offset, Function func) const {
  auto index = std::distance(m_frames.begin(), std::lower_bound(m_frames.begin(), m_frames.end(), frame));
  switch (m_record.type) {
  case Data::Type::Double : return func(static_cast<DataT<double> const*>(m_data.get())->data[index * m_record.items + offset]);
  case Data::Type::Float  : return func(static_cast<DataT<float > const*>(m_data.get())->data[index * m_record.items + offset]);
  case Data::Type::Int    : return func(static_cast<DataT<int   > const*>(m_data.get())->data[index * m_record.items + offset]);
  case Data::Type::Short  : return func(static_cast<DataT<short > const*>(m_data.get())->data[index * m_record.items + offset]);
  case Data::Type::Char   : return func(static_cast<DataT<char  > const*>(m_data.get())->data[index * m_record.items + offset]);
  default : throw std::logic_error("invalid Data::Type set in Channel");
  }
}

// SFINAE Catch-All Function. Prints descriptive error message in case of type deduction failure.
template <typename T> auto Channel::push_back(T)
-> std::enable_if_t<!std::is_arithmetic<T>::value> {
  static_assert(std::is_arithmetic<T>::value, "type T must be arithmetic");
}

template <typename T> auto Channel::push_back(int frame, T datum)
-> std::enable_if_t<std::is_arithmetic<T>::value> try {
  this->check_type<T>();
  m_frames.emplace_back(frame);
  this->get_data([&datum = datum](auto& m_data) { m_data.push_back(std::move(datum)); });
} catch (std::invalid_argument const& e) {
  throw std::invalid_argument(std::string(e.what()).append(" in Channel::push_back<T>(int, T)"));
}

template <typename T> auto Channel::push_back(int frame, std::vector<T> const& data)
-> std::enable_if_t<std::is_arithmetic<T>::value> try {
  this->check_type<T>();
  m_frames.emplace_back(frame);
  this->get_data([&](auto& m_data) { m_data.insert(m_data.end(), data.cbegin(), data.cend()); });
} catch (std::invalid_argument const& e) {
  throw std::invalid_argument(std::string(e.what()).append(" in Channel::push_back<T>(int, std::vector<T>)"));
}

template <typename T> void Channel::check_type() const {
  if (m_record.type != Data::get_type<T>()) throw std::invalid_argument("type T does not match internal type in Channel");
}

A note on attempted alternatives. I first tried a vector of pointers to polymorphic containers, but that was terribly slow (contiguity). Next, I moved the vector into the polymorphic container and used a pointer to that instead (current design, significant performance improvement), but struggled with interfacing with the underlying type. void* worked, but was dangerous and difficult to work with. Explicit casts (template <typename T> std::vector<T> const& data() const worked better, but it was too easy to bad-cast. That is how I ended up with the current solution: contiguous and type safe. — EraZ3712, Jun 24 '16 at 13:25

Quuxplusone · Accepted Answer · 2016-06-24 20:58:36Z

The first thing that stood out to me was the overuse and misuse of SFINAE:

// SFINAE Catch-All Function. Prints descriptive error message in case of type deduction failure.
template <typename T> auto Channel::push_back(T)
-> std::enable_if_t<!std::is_arithmetic<T>::value> {
  static_assert(std::is_arithmetic<T>::value, "type T must be arithmetic");
}

The above code doesn't make any sense. First of all, this use of enable_if would only make sense if you had another function with the signature push_back(T) enabled for arithmetic types; but you don't. What you've implemented here is a function push_back(T) that does not participate in overload resolution when T is arithmetic, and static-asserts when T is not arithmetic; i.e., it's completely useless.

Perhaps it was a typo, and you meant to have a frame parameter —

template <typename T> auto Channel::push_back(int /*frame*/, T /*datum*/)
-> std::enable_if_t<!std::is_arithmetic<T>::value> {
  static_assert(std::is_arithmetic<T>::value, "type T must be arithmetic");
}

so that this overload would pick up the calls to push_back(frame, datum) that didn't get picked up by the other SFINAE overload of that signature. However, in that case the SFINAE isn't buying you anything. The point of SFINAE is to make functions disappear, not to make them error out. If you just want them to error out, that's what static_assert is for. Remove this overload, and replace the other one with

template <typename T> auto Channel::push_back(int frame, T datum)
try {
  static_assert(std::is_arithmetic_v<T>, "type T must be arithmetic");  // Ta-da!
  this->check_type<T>();
  m_frames.emplace_back(frame);
  this->get_data([&datum = datum](auto& m_data) { m_data.push_back(std::move(datum)); });
} catch (std::invalid_argument const& e) {
  throw std::invalid_argument(std::string(e.what()).append(" in Channel::push_back<T>(int, T)"));
}

As @JanKorous said, it's very uncommon to see function-try-blocks in C++, except possibly in constructors (because the function-try-block also catches exceptions thrown from expressions in the member-initializer-list). From context I would guess that you're doing it so that your function body can have a single return, making static analysis easier. But yeah, I'd avoid function-try-blocks in production codebases, just because it's going to confuse everyone who comes after you (possibly including yourself in six months).

Your indentation style is a bit idiosyncratic. Consider using four-space indents (instead of two-space indents), and not cuddling so much stuff into the function header line. Personally, I prefer

template<typename T>
auto X::foo(T t) const
    -> decltype(t+1)
{
    return t+1;
}

whereas you seem to prefer

template<typename T> auto X::foo(T t) const -> decltype(t+1) {
    return t+1;
}

which in my opinion is just too much stuff on a single line.

It's not a question of columns-per-source-line; it's a question of ideas-per-source-line. One idea per source line is about right, IMHO. "Here comes a template... here's its signature... oh, and it has a surprising/SFINAE'd return type... okay, here comes the function body."

In get_data, you repeat over and over the pattern

static_cast<DataT<SOMETHING>*>(m_data.get())->data

That's deeply nested and therefore hard to parse. What if you separated it out into a helper method

template<typename T>
auto& get_data() const
{
    return static_cast<DataT<T> *>(m_data.get())->data;
}

? Then you could write simply

template <typename Function>
auto Channel::get_data(Function func)
{
    switch (m_record.type) {
        case Data::Type::Double: return func(get_data<double>());
        case Data::Type::Float: return func(get_data<float>());
        case Data::Type::Int: return func(get_data<int>());
        case Data::Type::Short: return func(get_data<short>());
        case Data::Type::Char: return func(get_data<char>());
        default: throw std::logic_error("invalid Data::Type set in Channel");
    }
}

In fact, you should read up on X Macros and consider whether the above code would be more readable and maintainable (particularly regarding what happens if you want to support unsigned types, long long, etc.) if you wrote it as

template <typename Function>
auto Channel::get_data(Function func)
{
    switch (m_record.type) {
#define X(ENUM, TYPE) \
        case Data::Type::ENUM: return func(get_data<TYPE>());
#include "supported_types.ipp"
#undef X
        default: throw std::logic_error("invalid Data::Type set in Channel");
    }
}

  // Data Interface Functions.
  template <typename T> decltype(DataT<T>::data) const& data() const;
  template <typename T> T const& datum(int frame, int offset) const;
  template <typename Function> auto data(Function func) const;
  template <typename Function> auto datum(int frame, int offset, Function func) const;

Having methods named both data and datum (as well as the above get_data) is asking for trouble. Overloading both names with two signatures apiece is practically demanding trouble. What's the point of passing Function func here, when you could just have the client code call their func on the return value of the non-Function-taking method?

decltype(m_frames) const& frames() const { return m_frames;        }

This would be much clearer if written simply

const auto& frames() const { return m_frames; }

It might also make sense to provide a non-const version, I'm not sure.

Instead of

  enum struct Type : char {
    Double = 'd'
  , Float  = 'f'
  , Int    = 'i'
  , Short  = 's'
  , Char   = 'c'
  };

consider using typeid directly; it would simplify a lot of your code if you could just say

typeid(char)

instead of

Data::Type::Char

and so on. Plus, you could more easily extend your code to deal with e.g. long long, without having to bikeshed over whether the enumerator value for Data::Type::LongLong (or is it Data::Type::Longlong?) should be 'l' or 'L' or...

boost::any's type() method deals directly in typeids, and it's always worked great for me.

I question your need for std::unique_ptr; it seems like you're putting an extra layer of stuff on the heap that you don't really need. But I haven't looked closely enough to be 100% sure.

Whoo, that's a lot! Thank you so much! You are correct, the SFINAE is unnecessary. It was leftovers from experimentation when I was learning how SFINAE worked, but as you said, it serves no good purpose in this code. I think I'll stick with 2-space indentation, but I'll be sure to apply your "one idea per line" idea. I think that is very good. Odd, I usually move repetitive code into a lambda, but seems I forgot to in this case (switch statements). I agree, there's too much going on in the switch statements. I would like avoid using macros though. (continued below). — EraZ3712, Jun 25 '16 at 1:20
I don't know how it slipped my mind to use auto for the return types instead of decltype(). It must haven from when I had defined the functions in a source file, which prevented auto deduction. The types that are used are guaranteed to be the 5 fundamental arithmetic types, so being able to handle additional types is not necessary. I'll consider your suggestion in future code. I avoided using "one for any" type containers such as union or any due to significant speed and memory performance drawbacks. See this Reddit comment I wrote concerning this topic. — EraZ3712, Jun 25 '16 at 1:28
Lastly, concerning the Function variations of both data() and datum(), it was to provide a safe means of accessing the underlying templated type without the need for explicit casts by the user. If the user wants to return the underlying type, they could do so using implicit casts by specifying a return type instead, avoiding the possibility of bad-casts. See this Reddit post for how I reached this solution. However, in the case where the user is confident in the underlying type, I also provide an option to access the data using explicit casts. — EraZ3712, Jun 25 '16 at 1:47

Jan Korous · Answer 2 · 2016-06-23 21:48:08Z

Very nice code that is inspiring to read! I have few minor nit-icks.

interface

I would personaly rather use specific types than decltype and auto in public interface. I guess users might appreciate that.

e. g. here

// Channel Interface Functions.
decltype(m_frames) const& frames() const { return m_frames;        }

or here

template <typename Function> auto data(Function func) const;

Data::Type conversion in Channel::create

To a large degree this code is inverse conversion to Data::get_type() and I would probably separate that logic out.

switch (record.type) {
case Data::Type::Double : return {std::make_unique<DataT<double>>(), std::move(record)};
case Data::Type::Float  : return {std::make_unique<DataT<float >>(), std::move(record)};
case Data::Type::Int    : return {std::make_unique<DataT<int   >>(), std::move(record)};
case Data::Type::Short  : return {std::make_unique<DataT<short >>(), std::move(record)};
case Data::Type::Char   : return {std::make_unique<DataT<char  >>(), std::move(record)};
default : throw std::invalid_argument("invalid Data::Type in Channel::create(Data::Type)");
}

function try block

Quite an exotic thing for non-constructor usage. Nothing wrong with this here but people maintaing the code might be surprised. It is not necessary though.

template <typename T> decltype(DataT<T>::data) const& Channel::data() const try {
  check_type<T>();
  return static_cast<DataT<T> const*>(m_data.get())->data;
} catch (std::invalid_argument const& e) {
  throw std::invalid_argument(std::string(e.what()).append(" in Channel::data<T>()"));
}

could be rewritten as

template <typename T> decltype(DataT<T>::data) const& Channel::data() const {
    try {
      check_type<T>();
      return static_cast<DataT<T> const*>(m_data.get())->data;
    } catch (std::invalid_argument const& e) {
      throw std::invalid_argument(std::string(e.what()).append(" in Channel::data<T>()"));
    }
}

I admit this is rather subjective.

Thank you for the response! (Whoops, didn't know enter submits the comment. Editing...) — EraZ3712, Jun 24 '16 at 12:44
Ack, you can only edit in 5 minutes? I like decltype for interface functions because changing the member variable's type propagates to its interface functions. The auto return type for data() and datum() is so that Function can have a non-void, non-deducible return type. I agree that the switch statements are doing the inverse of Data::get_type<T>(), but I couldn't find a way of separating the logic due to constant expression requirements. Do you have any suggestions? I use function try blocks to separate the exception handling code from intended code. Is it so strange? — EraZ3712, Jun 24 '16 at 13:05

asked	7 months ago
viewed	68 times
active	7 months ago

current community

your communities

more stack exchange communities

Data structure to store large numbers of runtime-dependent arithmetic types

Context

Code

2 Answers 2

interface

Data::Type conversion in Channel::create

function try block

Your Answer

Not the answer you're looking for? Browse other questions tagged c++ generics template c++14 polymorphism or ask your own question.

Hot Network Questions

current community

your communities

more stack exchange communities

Data structure to store large numbers of runtime-dependent arithmetic types

Context

Code

2 Answers 2

interface

Data::Type conversion in Channel::create

function try block

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c++ generics template c++14 polymorphism or ask your own question.

Related

Hot Network Questions