6
\$\begingroup\$

I made a template pipe and filters to replace an old pipe and filters implementation that used inheritance and had a very heavy base class.

#include <iostream>
#include <boost/bind.hpp>
#include <boost/function.hpp>
#include <list>

template <typename Data>
class pipeline {
    typedef boost::function<Data(Data)> filter_function;
    typedef std::list<filter_function> filter_list;
    filter_list m_list;

public:
    template<typename T>
    void add(T t) {
        m_list.push_back(boost::bind(&T::filter, t, _1));
    }

    Data run(Data data) {
        typedef typename filter_list::iterator iter;
        iter end = m_list.end();

        for(iter it = m_list.begin(); it != end; ++it) {
            data = (*it)(data);
        }

        return data;
    }
};

struct foo {
    int filter(int i) {
        return i + 1;
    }
};

int main(int argc, _TCHAR* argv[])
{
    pipeline<int> pipe;
    foo f;
    pipe.add(f);
    std::cout << pipe.run(0);
    char c;
    std::cin >> c;
}

Except the fact that add() is a template, are there any issues anyone sees with this approach?

\$\endgroup\$
  • \$\begingroup\$ The answers really depend on what C++ standard you have access to. With C++11, you could throw boost away. \$\endgroup\$ – Vorac Apr 22 '17 at 12:20
6
\$\begingroup\$
  • As run() is several lines long, it should be defined outside the class. Anything defined inside is automatically inlined.

  • Depending on the size of data when passing by value, it may be best to pass it to run() by const&, modify a local copy, and return that. RVO should still kick in.

  • Is there a significance to the name foo here? If not, it should be more accurate and start with a capital letter as it's a user-defined type. The capitalization also applies to pipeline.

  • iter end = m_list.end(); seems pointless here. You can just use m_list.end() inside the loop statement.

    As m_list is not being modified inside the loop, you can instead use cbegin() and cend() for const-correctness.

    If you have C++11, you can use auto to replace the defined iterator inside the loop statement.

  • Your "pause" is okay, but you can also use std::cin.get() to do the same thing.

\$\endgroup\$
  • \$\begingroup\$ start with a capital letter as it's a user-defined type. The capitalization also applies to pipeline. If you could provide a reference here, it would be a kick in the nuts for my colleagues, claiming that "if STL does it, it's correct". \$\endgroup\$ – Vorac Apr 22 '17 at 12:22
  • 1
    \$\begingroup\$ @Vorac: This answer might help, which also includes a quote from Stroustrup. \$\endgroup\$ – Jamal Apr 23 '17 at 2:31
1
\$\begingroup\$

Using std::vector instead of std::list may be more efficient (both for speed and memory allocation). Advantages are explained in answer to this question.

Just change

typedef std::list<filter_function> filter_list;

to

typedef std::vector<filter_function> filter_list;
\$\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.