3
\$\begingroup\$

The Go programming language has that slice type, which is under the hood a pointer to an actual array. This allows treating the subarrays as actual arrays with almost no performance overhead. This code snippet is about implementing slice type for C++ arrays and vectors.

slice.h:

#ifndef SLICE_H
#define SLICE_H

#include <sstream>
#include <stdexcept>
#include <string>

template<class T>
class slice {
    private:
        T*     m_array;
        size_t m_length;

    public:
        slice(T* array, size_t length) {
            this->m_array  = array;
            this->m_length = length;
        }

        slice(T* array, size_t length, size_t skip) {
            this->m_array = array + skip;
            this->m_length = length;
        }

        T& operator[](size_t index) { 
            if (index >= m_length) {
                std::ostringstream os;
                os << "Bad index: " << index << ", slice size: " << m_length;
                throw std::runtime_error(os.str());
            }

            return m_array[index];
        }

        size_t size() {
            return m_length;
        }

        T* begin() {
            return m_array;
        }

        T* end() {
            return m_array + m_length;
        }

        const T* begin() const {
            return m_array;
        }

        const T* end() const {
            return m_array + m_length;
        }
};

#endif  /* SLICE_H */

main.cpp:

#include <algorithm>
#include <iostream>
#include <vector>

#include "slice.h"

using std::cout;
using std::endl;
using std::vector;

int main(int argc, char** argv) {
    int int_array[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 };
    //// Should be 4, 5, 6, 7, 8.
    slice<int> pizza_slice = slice<int>(int_array, 5, 3);

    cout << "Arrays slice size: " << pizza_slice.size() << endl;

    for (int i : pizza_slice)
    {
        cout << i << endl;
    }

    vector<int> int_vector = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 };

    //// Should be 2, 3, 4
    pizza_slice = slice<int>(&int_vector[0], 3, 1);

    cout << "Vector slice size: " << pizza_slice.size() << endl;

    for (int i : pizza_slice) 
    {
        cout << i << endl;
    }

    return 0;
}

Any critique is much appreciated.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ You're missing an operator[] const; and don't you need to copy the slice if the user tries to modify it? Otherwise you end up modifying the original array as well as the slice. Right now all you've got is a slightly heavier-weight version of T*. \$\endgroup\$ – Quuxplusone Dec 29 '15 at 11:21
  • \$\begingroup\$ 90% of the functionality of go's slice is embedded in go's garbage collector. Go often creates new underlying arrays, and the slice itself gets copied with impunity - and the GC cleans up the mess. I don't see any memory management in your implementation at all. Without the append(...) and copy(...) functions, your slice is hardly "go like" \$\endgroup\$ – rolfl Dec 29 '15 at 14:26
  • \$\begingroup\$ Maybe I'm missing something, but isn't this basically just an iterator? \$\endgroup\$ – Dan Oberlam Dec 30 '15 at 1:53
5
\$\begingroup\$

This looks pretty good.

Use mem-initializer lists

When you write

slice(T* array, size_t length) {
    this->m_array  = array;
    this->m_length = length;
}

you're default constructing m_array and m_length and then assigning them. For pointers and integral types, this isn't exactly expensive, but it's a waste and may as well not get into that habit. Prefer directly constructing:

slice(T* array, size_t length)
: m_array(array)
, m_length(length)
{ }

Your other constructor can on the one hand just delegate to this one:

slice(T* array, size_t length, size_t skip)
: slice(array + skip, length)
{ }

but on the other hand doesn't really need to exist anyway. Just let the user pass in where he wants to start. This strikes me as just bloaty.

const functions

size() should be const, and you're missing T const& operator[](size_t) const.

You could also provide a T* data() and T const* data() const, which in this case just alias begin().

Additionally, consider providing a different constructor so that I can construct a slice<const T> from a slice<T>. That's something that makes sense.

throwing

If you're throwing due to out of range indexing, you should throw std::out_of_range. But generally, avoid having operator[] throw. This is what you're going to use most often so it's best to make it as simple as possible. If you want to provide a throwing alternative, the typical pattern followed by the standard library is to provide an at() function that does the bounds checking and then forwards along to operator[].

constexpr

Everything here can be constexpr. Just go overboard.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Started to answer this but you covered all the points already. Now I have nothing to do today :-( \$\endgroup\$ – Martin York Dec 29 '15 at 19:07
2
\$\begingroup\$

A good start, but you aren't going far enough:
Your type is basically a range, but limited to contiguous ones represented by pointers.

Though I wonder why you think a generic building-block has any business producing any output not explicitly requested?

Also, it would be far more comfortable if you used a maker-function.

If you generalize it, initializing your members with mem-initializers instead of assigning them in the ctor-body might make a difference.

Also, lots of your members aren't noexcept even though they never throw, nor constexpr even though they likely are, nor is size const even though it does not mutate.

As already mentioned, the proper way to complain about out-of-range indices is throwing std::out_of_range.
Though only in those cases where you should check them.

Consider using assert for additional checks which should only be done in the debug-build.

You are also missing member-functions cbegin(), cend(), and those for reverse iterators, a default-constructor for an empty range, member typedefs value_type, size_type, difference_type, reference, const_reference, pointer, const_pointer, reverse_iterator and const_reverse_iterator.


Regarding your test-suite:

  • Don't use std::endl unless you might need the explicit manual flush. If you don't need it, that needlessly kills your performance.
  • return 0; is implicit in main.
  • I'm not sure why you use a std::vector in your test-suite, but you probably have a good reason...
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.