5
\$\begingroup\$

Goal:

  • Wrap a vector. The wrapper can create view on part of the vector.
  • Support const_view and view.
  • view allows modification of elements while const_view does not.
  • The views are ranges (has begin() and end() methods).

Problem:

  • Code duplication between the const_view and view.

The actual code implements a 2D sparse array that saves more memory than usual. So I want to learn how to solve this problem.

Simplified Code (implements a dense 2D array)

#include <vector>
class A_csub;
class A_sub;

typedef std::vector<int> V;
typedef typename V::iterator It;
typedef typename V::const_iterator Cit;

class A {
// 2D dense array with _r rows and _c columns.
public:
    A(const int r, const int c);
    A_csub cview(const int n) const;
    A_sub view(const int n); // view nth row
    It sub_begin(const int n); // iterator at begin of nth row
    Cit sub_cbegin(const int n) const;
    int _r;
    int _c;
private:
    V _v;
};

class A_csub {
public:
    A_csub(const A& a, const int n);
    Cit begin() const;
    Cit end() const;
    const int size() const;
private:
    const A& _a;
    const int _n;
};

class A_sub {
public:
    A_sub(A& a, const int n);
    It begin();
    It end();
    const int size() const;
private:
    A& _a;
    const int _n;
};

// -- A -- //
A::A(const int r, const int c) : _r(c), _c(c), _v(r*c) {}
A_csub A::cview(const int n) const { return A_csub(*this, n); }
A_sub A::view(const int n) { return A_sub(*this, n); }
It A::sub_begin(const int n) { return _v.begin() + n*_r; }
Cit A::sub_cbegin(const int n) const { return _v.cbegin() + n*_r; }

// -- A_csub -- //
A_csub::A_csub(const A& a, const int n) : _a(a), _n(n) {}
Cit A_csub::begin() const { return _a.sub_cbegin(_n); }
Cit A_csub::end() const { return begin() + _a._r; }
const int A_csub::size() const { return _a._r; }

// -- A_sub -- //
A_sub::A_sub(A& a, const int n) : _a(a), _n(n) {}
It A_sub::begin() { return _a.sub_begin(_n); }
It A_sub::end() { return begin() + _a._r; }
const int A_sub::size() const { return _a._r; }

int main() {
    A a(10,5);
    a.cview(4);
    A_sub b = a.view(4);
    for (auto && e:b) {e=1;}
}
\$\endgroup\$
1
  • \$\begingroup\$ +1 Hi again. A good decision to move your question here. I hope you'll get good answers for that code review. The time taken might be longer than receiving answers at SO, but the quality is usually better. \$\endgroup\$ Feb 3, 2017 at 17:43

1 Answer 1

2
\$\begingroup\$

The variable names you chose are quite bad. I can't seem to figure out what r and c are as constructor input.

Creating such views only for vectors (of ints) is a bit wasteful. You are basically wrapping an iterator pair, so why not make the code generic so that it woks with iterators of any type of container?

Boost, for example, provides Boost.Range for doing just this.

When you implemented a specialized container, you create specialized iterators that know how to traverse it. The view/range stays the same, it's job is still to wrap a pair of iterators.

\$\endgroup\$
4
  • \$\begingroup\$ That makes sense. Probably still need to create those views. If I want to view each row. I didn't write iterator that returns a view. I guess if I use Boost.Range, I would need to create (const) view class and iterator that returns the (const) view class. Is that true? I want a view on a row because sometimes I want to work over the whole row. \$\endgroup\$ Feb 3, 2017 at 18:36
  • \$\begingroup\$ May be I should make the sample code even simpler. Just view and const view on the nth element of a 1D array, and ask how to remove code duplication. \$\endgroup\$ Feb 3, 2017 at 18:42
  • 1
    \$\begingroup\$ The key is in the iterators. You provide a way to get an iterator for the begging of the row and one for immediately past the end of the row. That way, any code that uses those iterators traverses only that one row, without needing to know any details of what a row in your collection actually is. \$\endgroup\$
    – D. Jurcau
    Feb 3, 2017 at 18:45
  • \$\begingroup\$ Turns out that I can't use iterator because openmp doesn't work well with it. stackoverflow.com/questions/2513988/… \$\endgroup\$ Feb 3, 2017 at 20:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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