Arduino C++ class with arrays

You need to allocate some memory if you're going to use this approach.

_bindings = new short[size];

Don't forget to free this memory whenever you are done with it.

delete[] _bindings;

Dynamic array in C++ is found under the name vector. You call would become:

class Relay
{
    public:
        Relay(const std::vector<short> &pins) 
           : _binding(pins.size()), _relay(pins) {}
    private:
        std::vector<short> _relay;
        std::vector<short> _binding;
};


int main() {
  // C++ 11
  //  Relay r({1, 2, 3, 4});

  // C++03
  short arr[] = {1, 2, 3, 4};
  std::vector<short> v(std::begin(arr), std::end(arr));
  Relay r(v);
}

There are multiple issues in your code, for instance:

short *_relay;

is a pointer, sizeof(_relay) is the size of the pointer, nothing to do with the size of the memory allocation it points to. So sizeof(_relay)/sizeof(short); is equivalent to sizeof(short *)/sizeof(short); which does not really make any sense.

When you do:

_relay = _pins;

you are copying a pointer, now _relay points to the same place _pins point. However this memory where they points to was "volatile": it is a temporary you allocated at the call site. It will be de-allocated as soon as the call return. Your pointer is now pointing to an area you should not access anymore.

It's generally questionable class design to have a C++ class contain a pointer to "outside data." Often a better way to do it is to have your C++ use a copy of the external data, so it's clear that the memory is de-allocated when the class instance is destroyed.

Also as someone else mentioned, some of these things are much easier to do with C++11. If you're using g++ or clang++ as your compiler, you may already have support for C++11, which makes a lot of things very nice and easy, especially for embedded system work.

In this particular case, here's a complete and compilable program (I used g++ and compiled with g++ -std=c++11 -o relay relay.cpp with the following as the contents of relay.cpp. I've combined everything into a single file for simplicity of illustration, but in real life, you should keep the separation of .cpp and .h files that you already had.

#include #include

class Relay
{
    public:
        Relay(std::vector<short> pins)
            : _relay(pins), _binding(pins.size()) {}
        // these are just diagnostics for testing the class
        std::ostream& printPins(std::ostream &out) {
                for (auto i : _relay) 
                      out << i << ' '; 
                   out << std::endl; 
        }
        std::ostream& printBindings(std::ostream &out) { 
                for (auto i : _binding) 
                    out << i << ' '; 
                out << std::endl; 
        }
    private:
        std::vector<short> _relay;
        std::vector<short > _binding;
};

enum {A0=80, A1, A2, A3, A4};

int main()
{
    Relay reles{std::vector<short>{11, 10, 9, 8, 7, 3, 2, 73, 
                                 4, A0, A1, A2, A3, A4}};
    reles.printPins(std::cout);
    reles.printBindings(std::cout);
    return 0;
}

The enum for the A0 through A4 lines are just for completeness in this example, but they might be #define or const int declarations. It doesn't matter.

The two print...() functions are only for illustration to demonstrate that it's actually doing what's expected. In this case, the output is:

11 10 9 8 7 3 2 73 4 80 81 82 83 84 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 

The std::vector is part of the standard library and could be used even if you're not using C++11. If you are, then you can take advantage of such interesting things as move semantics and constexpr and other goodies that are of particular use in embedded systems. One such nice feature is called a list initialization and is used within main in this sample program. In this line:

    Relay reles{std::vector<short>{11, 10, 9, 8, 7, 3, 2, 73, 4, A0, A1, A2, A3, A4}};

A temporary std::vector<short> is created and then used to initialize the reles class instantiation. A clever compiler can, under some circumstances, optimize away the creation of such temporaries. As always, looking at the actual assembly language output is useful for evaluating particular techniques and particular compilers.

Another thing that I find very useful is the for (auto i : _relay) style of for loop. This tells the compiler to automatically infer the type for i by treating relay as a collection of something. Very concise and very handy.

If you're not yet familiar with the C++ language (or just the relatively new parts), I'd recommend getting a book such as Stroustrup's The C++ Programming Language, fourth ed..

Edit: In the unfortunate case that you have a very limited C++ compiler, as seems to be the the situation with the Arduino, you'll need to do things differently. Specifically, because you don't have an operator new or operator delete you'll have to either pre-allocate some maximum size for your arrays, or simply rely on those being passed in as part of the initialization. Here's one way to do that:

#include <cassert>

class Relay
{
    public:
        Relay(int numpins, short *pins, short *bindings)
            : _numpins(numpins), _relay(pins), _binding(bindings) {}
    short pin(int i) 
    {
    if ((i < 0) || (i >= _numpins))
        return -1;
    return _relay[i];
    }
    short binding(int i)
    {
    if ((i < 0) || (i >= _numpins))
        return -1;
    return _binding[i];
    }
    private:
        int _numpins;
        short *_relay;
        short *_binding;
};

enum {A0=80, A1, A2, A3, A4};

int main()
{
    const int numpins = 14;
    short pins[numpins] = {11, 10, 9, 8, 7, 3, 2, 73, 
                           4, A0, A1, A2, A3, A4};
    short bindings[numpins] = {1, 2, 3, 4, 5, 6, 7,
                           8, 9, 10, 11, 12, 13, 14};
    Relay reles(numpins, pins, bindings);
    assert(reles.pin(0) == 11);
    assert(reles.binding(4) == 5);

    return 0;
}