user2058002
user2058002

Reputation:

Encapsulating a vector

Here is a small sample of my class:

#include <string>
#include <vector>

using std::string;
using std::vector;

struct Book {
  string m_author;
  string m_title;
};

class BookList
{
public:
  BookList();
  ~BookList();

private:
  vector<Book*> m_books;
}

As you can see, the data for BookList is stored in the form of a vector of Books. What would be the best way to write an accessor? Should I allow the user to retrieve them one by one via HasMore() & GetNextBook() methods, or would it be better to just return the entire vector? Or perhaps an iterator?

Thanks in advance.

Upvotes: 2

Views: 1648

Answers (5)

Andrew Durward
Andrew Durward

Reputation: 3861

As others have mentioned, removing the BookList class and replacing it with an appropriate typedef is probably the best option if BookList has no other responsibilities. If however, BookList is required then some options to provide access to m_books would be:

  • a method that returns a vector< Book* > const &
  • a pair of begin/end methods that return polymorphic iterator wrappers such as this or this

The first option is the simplest but exposes the internal data layout to clients. The second option would allow you to replace the underlying container without requiring any changes to clients. It's up to you to decide which is most appropriate for your design.

Also, you may want to consider replacing vector< Book* > with vector< Book > if Book will not be a polymorphic base class. If it is, then a container of smart pointers would probably be a better option.

Upvotes: 0

hmjd
hmjd

Reputation: 121981

If you are not adding any additional features to the class you may consider just using a typedef instead:

typedef vector<Book*> BookList;

If you are adding additional features then expose iterators begin() and end(), similar to the STL containers, plus modification methods.

Upvotes: 1

Cat Plus Plus
Cat Plus Plus

Reputation: 129814

First of all, you probably don't need to store a pointer. Use a value instead. And here you have one of three options, really:

  1. Return a vector, by value or by reference.
  2. Expose begin/end iterators.
  3. Just make the vector public, or get rid of BookList altogether, and just use std::vector<Book> directly.

GetNextBook approach is bad, because it makes your list store iteration state for no good reason. And also makes you the only person who does that.

Upvotes: 1

slugonamission
slugonamission

Reputation: 9642

Ultimately, you want to make this as abstract as possible, at least to anything public-facing. There's a few things to think of:

  • If you return the whole thing, you risk the public being able to modify the vector. Do you really want them to be able to do that?
  • What if you need to change the layout of the class at some point? If you write your own methods to return the required data, it allows you to swap out the vector at some point in the future without killing compatibility with everything else.
  • Returning the whole vector will make a massive stack frame. You'd need to pass a pointer ideally.

If you probably won't need to change the implementation, returning an iterator could work.

Upvotes: 0

piokuc
piokuc

Reputation: 26184

Return reference to the vector or pair of iterators. Writing accessors / reimplementing iterators interface in a non-standard way is pointless. Another option, useful sometimes, is to derive your class from an STL container.

Upvotes: 0

Related Questions