0

I'm designing a class which has two standard vectors as members. I would like to be able to use range-based for loops on the vector elements and I came up with this solution

#include <iostream>
#include <vector>

using namespace std;

class MyClass {
public:
  void addValue1(int val){data1_.push_back(val);}
  void addValue2(int val){data2_.push_back(val);}
  vector<int> const & data1() const {return data1_;}
  vector<int> const & data2() const {return data2_;}
  // ...
private:
  vector<int> data1_;
  vector<int> data2_;
  // ...
};

void print1(MyClass const & mc) {
  for (auto val : mc.data1()){
    cout << val << endl;
  }
}

void print2(MyClass const & mc) {
  for (auto val : mc.data2()){
    cout << val << endl;
  }
}

int main(){
  MyClass mc;
  mc.addValue1(1);
  mc.addValue1(2);
  mc.addValue1(3);
  print1(mc);
}

Clearly, the alternative of defining begin() and end() functions doesn't make sense since I have two distinct vectors.

I would like to ask the following questions:

  • A shortcoming of the proposed solution is that the contents of the two vectors cannot be changed (due to the const qualifier). In the case I need to modify the vector elements how can I modify the code? EDIT: the modification should preserve encapsulation

  • Considering data encapsulation, do you think it is bad practice to return a (const) reference to the two vectors?

carlo
  • 325
  • 1
  • 4
  • 12
  • "Modifying the contents of the vectors" **by definition** is unencapsulating them. That's what encapsulation **is**. And thus the simple answer becomes: `struct MyClass { vector data1; vector data2; }` – Caleth Nov 28 '17 at 10:07
  • @Caleth : In my question I wrote that I want to be able to change the value of the vector elements but I do not want to give the ability of adding and removing elements. From my understanding encapsulation requires that only certain safe operation can be performed on the data (i.e. for my application writing and reading element values) while other operations are hidden (i.e. for my application adding and removing data). – carlo Nov 28 '17 at 14:57
  • Oh, in which case you definitely want @Yakk's solution. Not mentioning `vector` in the public members of MyClass is "more encapsulated" – Caleth Nov 28 '17 at 15:06

2 Answers2

3

Use something like gsl::span<int> and gsl::span<const int>.

Here is a minimal one:

template<class T>
struct span {
  T* b = 0; T* e = 0;
  T* begin() const { return b; }
  T* end() const { return e; }
  span( T* s, T* f ):b(s),e(f) {}
  span( T* s, std::size_t len ):span(s, s+len) {}
  template<std::size_t N>
  span( T(&arr)[N] ):span(arr, N) {}
  // todo: ctor from containers with .data() and .size()

  // useful helpers:
  std::size_t size() const { return end()-begin(); }
  bool empty() const { return size()==0; }
  T& operator[](std::size_t i) const { return begin()[i]; }
  T& front() const { return *begin(); }
  T& back() const { return *(std::prev(end())); }

  // I like explicit defaults of these:
  span() = default;
  span(span const&) = default;
  span& operator=(span const&) = default;
  ~span() = default;
};

now you can write:

span<int const> data1() const {return {data1_.data(), data1_.size()};}
span<int const> data2() const {data2_.data(), data2_.size()};}
span<int> data1() {return {data1_.data(), data1_.size()};}
span<int> data2() {data2_.data(), data2_.size()};}
Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
-1

A shortcoming of the proposed solution is that the contents of the two vectors cannot be changed (due to the const qualifier). In the case I need to modify the vector elements how can I modify the code?

First of all, you should add a data1() and a data2() not-const versions that return a reference to the data1_ and data2_ members

vector<int> const & data1() const {return data1_;}
vector<int> const & data2() const {return data2_;}

vector<int> & data1() {return data1_;}
vector<int> & data2() {return data2_;}

Second: if you want modify the element in print1() (by example) you have to receive mc as not const reference

// ..........vvvvvvvvv   no more const
void print1 (MyClass & mc) {

so you can change mc.

Third: in the range based loop you have to define val as reference so you can modify it modifying also the referenced value inside the vector

// ........V   by reference
for ( auto & val : mc.data1() ) {
   ++val ;  // this modify the value in the vector inside mc
   cout << val << endl;
}

Considering data encapsulation, do you think it is bad practice to return a (const) reference to the two vectors?

IMHO: if the reference is const, not at all: it's a good practice because permit the safe use of the member without the need to duplicate it.

If the reference isn't const, I don't see big difference with declaring the member public.

max66
  • 65,235
  • 10
  • 71
  • 111
  • Maybe my question was not clear, but I wanted to have good encapsulation also after the code modification. I edited my question to make this clear. Thank you for answering to my second question. – carlo Nov 28 '17 at 08:30