Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I want to write a class which offers two sequences of elements to its users. The first one (lets call it "primary") is the main of the class and will be use 80% of the time. The second one (lets call it "secondary") is less important, but still need to be in the same class. The question is: what interface should the class offer to its users?

By looking at STL style, a class with a single sequence of elements should offer begin() and end() functions for traversal and function like insert() and erase() for modifications.

But how should my class offer the second sequence?

For now, I have two ideas:

  • Expose the two containers to the user (what about the Law of Demeter ?)
  • Provide the main container with STL interface and expose only the second one.

Here is an example.

#include <vector>

class A { 
    public:
        std::vector<int>&  primary();
        std::vector<char>& secondary();

    private:
        std::vector<int>  m_primary;
        std::vector<char> m_secondary;
};

class B { 
    public:
        std::vector<int>::iterator begin();
        std::vector<int>::iterator end();
        std::vector<char>& secondary();

    private:
        std::vector<int>  m_primary;
        std::vector<char> m_secondary;
};

// Classes implementation
// ...

int main() {

    // --------------------------------------------------
    // Case 1
    // --------------------------------------------------

    A a;

    for(auto it = a.primary().begin(); it != a.primary().end(); ++it) {
        // ...
    }   
    for(auto it = a.secondary().begin(); it != a.secondary().end(); ++it) {
        // ...
    }   

    // --------------------------------------------------
    // Case 2
    // --------------------------------------------------

    B b;

    for(auto it = b.begin(); it != b.end(); ++it) {
        // ...
    }   
    for(auto it = b.secondary().begin(); it != b.secondary().end(); ++it) {
        // ...
    }   
}

What is the more C++ish way to do that? Is one best than the other or is there an other solution?

Context

This problem came in the context of an exercise in which I am writing a simple database access framework. Here are the classes involved in the question:

  • table
  • column
  • row
  • field

The table class consists of a sequence of columns and an other sequence of rows. The main use of the table is manipulating (access, add and remove) the rows.

Deeper in the hierarchy, a row is made of column and field so a user can ask the value (field) corresponding to a given column or column name. Each time a column is add/modify/remove from the table, every rows will need to be modify to reflect the modification.

I want the interface to be simple, extensible and combining well with existing code (like STL or Boost).

share|improve this question
    
0: (no vote) Your question in current form is not specific enough to elicit answers that would be useful to other programmers. Please describe (i) What relations/constraints/invariants exist between the elements in the two sequences (and the rules that govern the co-modification of the two sequences) ; and (ii) how the users will be traversing the elements - sequential or random access? Absent any additional information, I suggest std::pair < std::vector < typename A> , std::vector < typename B > > as the simplest type that answers your question. –  rwong Dec 11 '11 at 22:53
    
@rwong I've just add a "Context" section where I explain the relationship between the different elements. –  authchir Dec 12 '11 at 4:24

4 Answers 4

up vote 3 down vote accepted

Don't expose your guts, guide visitors :

class A { 
    public:

        // we assume you want read-only versions, if not you can add non-const versions
        template< class Func >
        void for_each_primary( Func f ) const { for_each_value( f, m_primary ); }

        template< class Func >
        void for_each_secodary( Func f ) const { for_each_value( f, m_secondary ); }

    private:
        std::vector<int>  m_primary;
        std::vector<char> m_secondary;

        template< class Func, class Container >
        void for_each_value( Func f, const Container& c )
        {
             for( auto i : c )
                 f( i );
        }
};



int main()
{
    A a;
    a.for_each_primary( [&]( int value ) 
         { std::cout << "Primary Value : " << value ; } );
    a.for_each_secondary( [&]( int value ) 
         { std::cout << "Secondary Value : " << value ; } )
}

Note that you could use std::function instead of template parameter if you want to put the implementation in a cpp file, making implementation changes less expensive on compilation times in big projects.

Also, I didn't try to compile it now, but I used a lot this pattern in my open-source projects.


This solution is a C++11 enhancement of B I guess.

HOWEVER

This solution have several issues :

  1. It requires C++11 to be effective, because it's efficient for the user of you class ONLY if he can use lambda.
  2. It relies on the fact that the class implementer really know what algorithms precisely are to be available to users. If the user need to do complex manipulations to the numbers, jumping from index to index in an unpredictable way for example, then exposing iterators, a copy of the values OR the values would be better.

In fact, this kind of choice totally depends on what you intend the user to do with this class.

By default I prefer the solution I gave you because it's the most "isolated" one, making sure the class know how it's values can be manipulated by external code. It's a bit like "extensions points". If it's a map, providing a find function to your class is easy. So I think that's the more sane way to expose data and it's also made available by lambdas.

As said, if you need to make sure the user can manipulate the data as he wish, then providing a copy of the container is the next "isolated" option (maybe with a way to reset the container with the copy after that). If a copy would be expensive, then iterators would be better. If not enough then a reference is acceptable but it's certainly a bad idea.


Now assuming you're using C++11 and don't want to provide algorithms, the most idiomatic way is using iterators this way (only the user code changes) :

class B { 
    private:
        std::vector<int>  m_primary;
        std::vector<char> m_secondary;
    public:

        // your code is read-write enabled... make sure it's not const_iterator you want
        // also I'm using decltypt to allow changing container type without having to manually change functions signatures
        decltype(m_primary)::iterator primary_begin() const;
        decltype(m_primary)::iterator primary_end() const;

        decltype(m_secondary)::iterator secondary_begin() const;
        decltype(m_secondary)::iterator secondary_end() const; 


};

int main()
{
    B b;


    std::for_each( b.primary_begin(), b.primary_end(), []( int& value ) {
        // ...
    });   
    std::for_each( b.secondary_begin(), b.secondary_end(), []( double& value ) {
        // ...
    });   

}
share|improve this answer

Since you are effectively re-implementing Boost.MultiIndex and most C++ developers should be at least vaguely familiar with Boost libraries, consider duplicating their interface. They use one index object per index and container which publishes begin() and end():

class A {
    struct primary {
        struct iterator;
        iterator begin();
        iterator end();
    };
    struct secondary {
        struct iterator;
        iterator begin();
        iterator end();
    };
    primary primary_view();
    secondary secondary_view();
};

One of the advantages is that you can give the view objects to any function expecting containers or boost ranges.

share|improve this answer
    
What is the difference between returning these struct and returning directly the container? –  authchir Dec 12 '11 at 17:32
    
@authchir: The views can have a reduced interface with respect to the original containers. Returning a const reference to a vector will provide the full vector interface, motivating users of the class to use it. When you want to change the implementation later, you'll have to provide the full vector interface. In contrast, the views can have much reduced functionality, e.g. have their iterators adapted to forward-only iterators. In short, the views encapsulate the underlying containers. –  thiton Dec 12 '11 at 17:37

Note that many STL containers have a second interface to iterate in the reverse direction: rbegin and rend!

So there is prior art to use begin() and end() for the primary interface, and e.g. sec_begin() and sec_end() for the secondary interface.

Something like (typedefs added for increased readability):

class B { 
    public:
        typedef std::vector<int>::iterator iterator;
        typedef std::vector<char>::iterator secondary_iterator;

        iterator begin() { return m_primary.begin(); }
        iterator end()   { return m_primary.end(); }

        secondary_iterator sec_begin() { return m_secondary.begin(); }
        secondary_iterator sec_end()   { return m_secondary.end(); }

    private:
        std::vector<int>  m_primary;
        std::vector<char> m_secondary;
};

Additional remark: Note that non-const iterators are being returned. If you really want both containers to act synchronized (that is: changing one should change the other), it might be better to return the const_iterator versions. begin()/end()/... can become const member functions.

The typedefs I added will help switching if you decide to do so.

share|improve this answer

A proper way would be to implement like this :

    #include <vector>
    struct A { 
        std::vector<int>  m_primary;
        std::vector<char> m_secondary;
    };

int main()
{
  A a;
  for ( auto & it : a.m_primary )
  {
  }
  for ( auto & it : a.m_secondary )
  {
  }
}

As you can see, you do not need an interface for such classes.

share|improve this answer
    
-1: Usually both containers are coupled: there is a reason why they are grouped in the same class! Providing public access makes it possible to corrupt the class invariants. –  Sjoerd Dec 12 '11 at 10:22
    
@Sjoerd c++ provides many ways to shoot yourself into the foot. It is very easy to corrupt the class invariants event if you provide begin/end (as in your answer), if that is what you want to do :) –  BЈовић Dec 12 '11 at 11:15

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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