Return row or column from a 2D array

Question

I have a 2D array class that is actually mapped to a 1D vector. For example, to obtain an element at a particular position, the code is as follows:

template <typename T>
T mat <T>::at(const size_t &row, const size_t &col) const
{
 return m_values[row*ncol() + col];
}

Here's the relevant section of the class in-context:

template <typename T>
class mat
{
private:
 std::vector <T> m_values;
 size_t m_nrow, m_ncol;
public:
 mat();
 size_t ncol() const{ return m_ncol; };
 size_t nrow() const{ return m_nrow; };
 T at(const size_t &row, const size_t &col) const;
 std::vector <T> row(const size_t &row) const;
 std::vector <T> col(const size_t &col) const;
};

m_nrow and m_ncol contain the number of rows and columns of the 2D array, respectively, while m_values contains the values of the array. They're assigned in another (long) constructor.

I now need to be able to return an entire row or column. Here's how that's currently implemented:

template <typename T>
std::vector <T> mat <T>::row(const size_t &row) const
{
 std::vector <T> tmp;
 for(size_t j=0; j < ncol(); j++)
 {
 tmp.push_back(at(row, j));
 }
 return tmp;
}
template <typename T>
std::vector <T> mat <T>::col(const size_t &col) const
{
 std::vector <T> tmp;
 for(size_t i=0; i < nrow(); i++)
 {
 tmp.push_back(at(i, col));
 }
 return tmp;
}

These implementations are easy to reason about but not particularly efficient: there's the pushing back to a temporary vector and the repeated calculation of row*ncol() in at(). Is there a more idiomatic way of doing this in C++11?

• Barry: Please post complete code. What is m_values? What is mat? What are ncol()and nrow()? These are very relevant. Stub code is off-topic here.
• Loki Astari: What do you expect to do with a row (or column). If the only thing is check size and accesses elements then a simple wrapper class that accesses the parent seems more appropriate.

Answer 1

Lack of modifiability

Your at(), row() and col() return copies. This means that once you construct your mat, it's permanently const. Also, for some Ts, this makes these functions unnecessarily expensive.

Prefer instead for at() to return a reference:

T& at(size_t row, size_t col);
T const& at(size_t row, size_t col) const;

Although, typically with the standard library, the at() member functions also do range checking. So consider some other non-throwing indexing mechanism (maybe operator()) and have at() defer to that one while doing bounds checking. Note that you don't need to pass the size_ts by reference-to-const, value is fine.

Now, for the other two, prefer instead:

std::vector<std::reference_wrapper<T>> row(size_t );
std::vector<std::reference_wrapper<const T>> row(size_t ) const;
std::vector<std::reference_wrapper<T>> col(size_t );
std::vector<std::reference_wrapper<const T>> col(size_t ) const;

reference_wrapper<T> since you can't have vector<T&>.

Constructing those rows/cols

Since you're storing everything in row order, returning a row can just use the iterator-pair constructor of std::vector:

std::vector<std::reference_wrapper<T>> row(size_t r)
{
 auto start = m_values.begin() + r * ncol();
 return {start, start + ncol()};
}

Returning a column is much more annoying, but you definitely want to use reserve() to avoid any extra allocations:

std::vector<std::reference_wrapper<T>> col(size_t c)
{
 std::vector<std::reference_wrapper<T>> result;
 result.reserve(nrows());
 for (size_t r = 0; r < nrows(); ++r) {
 result.push_back((*this)(r, c));
 }
 return result;
}

Where I'm assuming operator() is the non-throwing version of at.

• ruser45381: I certainly agree with adding bounds checking to the at() function. Can you explain why I shouldn't pass the size_ts by reference-to-const? I am using const in that situation to signal that the size_ts should not be modified in the body of the function. I also agree that I should reserve() the size of the vector.
• user2296177: @ruser45381 It might actually be more expensive to pass small types by reference than by copy. You can/should keep it const to indicate that it will not be changed, but removing the reference part is a good idea.
• ruser45381: That's good to know.
• Loki Astari: @user2296177: Your argument for that? Just becuase it is smaller does not mean it is cheaper. Below a certain point a register is a register is a register no matter what the size.
• user2296177: @LokiAstari I used this: stackoverflow.com/a/27260446/2296177. You seem to be correct in that it may not actually matter.