Home > Mobile >  Is there a way to extract outer loops of multiple similar functions?
Is there a way to extract outer loops of multiple similar functions?

Time:12-31

Example: I want to extract the nested for loops from these operator functions that are the same except the one line.

// Add two matrices
Matrix& operator =(const Matrix& other)
{
    for (int i = 0; i < this->m_rows; i  )
    {
        for (int j = 0; j < this->m_cols; j  )
        {
            (*this)(i, j) = (*this)(i, j)   other(i, j); // Only difference
        }
    }
    return *this;
}

// Subtract two matrices
Matrix& operator-=(const Matrix& other)
{   
    for (int i = 0; i < this->m_rows; i  )
    {
        for (int j = 0; j < this->m_cols; j  )
        {
            (*this)(i, j) = (*this)(i, j) - other(i, j); // Only different
        }
    }
    return *this;
}

CodePudding user response:

You can write a function template that accepts a binary function and applies it on all pairs of elements inside the loops

template<typename Op>
void loops(const Matrix& other, Op op)
{
    for (int i = 0; i < this->m_rows; i  )
    {
        for (int j = 0; j < this->m_cols; j  )
        {
            (*this)(i, j) = op((*this)(i, j), other(i, j)); 
        }
    }
}

and then use it like this

// Add two matrices
Matrix& operator =(const Matrix& other)
{
    loops(other, std::plus{});
    return *this;
}

// Subtract two matrices
Matrix& operator-=(const Matrix& other)
{   
    loops(other, std::minus{});
    return *this;
}

CodePudding user response:

I'd argue that this sort of problem often implies a poor abstraction.

In the example case, an efficient matrix will have a single contiguous array, where matrix(i,j) is translated to array[i*n_columns j] behind the scenes. The i,j interface is simpler in many cases (otherwise you'd just use a vector), but there's no reason to restrict the user accessing the underlying array elements directly - let alone within your own matrix class!

Another way to put it is that you're using classes that create the (i,j) abstraction for you, and now you're wanting another layer of abstraction to undo the work. This is costly in cpu time and in your time, and makes for spaghetti code. Instead, make sure that you (and your user) has access to the underlying array by direct element access and by iterator:

public:

auto & operator [] (size_t i)
{
    return data[i]; // our underlying array
}

const auto & operator [] (size_t i) const
{
    return data[i];
}

Matrix& operator  = (const Matrix& other)
{
    for (size_t i = 0; i < size();   i)
        data[i]  = other[i];

    return *this;
}

Matrix& operator -= (const Matrix& other)
{   
    for (size_t i = 0; i < size();   i)
        data[i] -= other[i];

    return *this;
}

You may think that this somehow breaks encapsulation, but it doesn't. The user has the same access to edit elements, but no access to change the matrix size or access to the raw pointer. However, for the sake of answering your more general question about avoiding loops, I'll pretend we only have access to the iterators:

Matrix& operator  = (const Matrix& other)
{
    auto it1 = begin();
    auto it2 = other.begin();

    for (size_t i = 0; i < size();   i)
        it1[i]  = it2[i];

    return *this;
}

Matrix& operator -= (const Matrix& other)
{   
    auto it1 = begin();
    auto it2 = other.begin();

    for (size_t i = 0; i < size();   i)
        it1[i] -= it2[i];

    return *this;
}

Okay, that's better, but we can abstract the repetitive part for any iterator or container, like so:

template <class Func, class ... Its>
void ForArrays (size_t size, Func&& f, Its && ... its)
{
    for (size_t i = 0; i < size;   i)
        f(its[i]...);
}

template <class Func, class ... Cs>
void ForContainers (Func&& f, Cs && ... cs)
{
    size_t size = (cs.size(), ...);

    assert(((size == cs.size()) && ...));

    ForArrays(size, f, cs.begin()...);
}

Demo

And now we can rewrite our operators:

Matrix& operator  = (const Matrix& other)
{   
    ForContainers([](auto& lhs, auto rhs){lhs  = rhs}, *this, other);

    return *this;
}

Matrix& operator -= (const Matrix& other)
{   
    ForContainers([](auto& lhs, auto rhs){lhs -= rhs}, *this, other);

    return *this;
}

I think the lesson here is to never undo the work of an abstraction; instead, if it's not useful for part of your code then avoid it entirely (in that part of the code). A layer of abstraction to undo a layer of abstraction is why some people turn away from OOP.

  • Related