Avoiding duplicate code for begin() and end() functions in generic containers

Question

There are several similar threads Q1 or Q2 to this one, but not exactly.

The problem is simply to write the following code in a elegant way, without code duplication:

template <typename T, class Container = std::vector<T> >
class container{
    iterator begin(){
        return iterator(data_.begin(), 1);
    }

    const_iterator begin() const{
        return const_iterator(data_.begin(), 1);
    }
    Container data_;
};

Q2 has an elegant way to avoid code duplicate for const and non-const function that have the exactly same signature. This is however not the case here because there is no cast to turn a const_iterator to an iterator.

Q1 provides a good way using templates, however, once the container is a class, the begin_impl function becomes static and at the same time friend.

The most elegant code I have come up with is:

template <T>
class container{
        template< typename I, typename C >
        static I begin_impl( C & c ){
            return I(data_.begin(), 1);
        }

        template< typename I, typename C >
        friend I container<T>::begin_impl( C & c );

        iterator begin(){
            return container<T>::template begin_impl< iterator >( *this );
        }

        const_iterator begin() const{
            return container<T>::template begin_impl< const_iterator >( *this );
        }
}

My question is whether this is the most elegant way in your opinion. If no, please suggest some better code.

EDIT: my iterator implementation:

class container{
        template <bool isConst>
        class iterator_ {

        public:
            typedef Container container_type;
            typedef typename Container::value_type value_type; 
            typedef typename Container::difference_type difference_type;
            typedef typename Container::size_type size_type;
            typedef typename Container::reference reference;
            typedef typename Container::const_reference const_reference;
            typedef typename Container::pointer pointer; 
            typedef typename Container::const_pointer const_pointer;
            typedef typename std::forward_iterator_tag iterator_category;

            template<bool isCond, typename cref, typename ref>
            struct IS_CONST_REF{
                typedef ref reference_type;
            };
            template<typename cref, typename ref>
            struct IS_CONST_REF<true, cref, ref>{
                typedef cref reference_type;
            };            
            typedef typename IS_CONST_REF<isConst, const_reference, reference>::reference_type ref;

            template<bool isCond, typename citr, typename itr>
            struct IS_CONST_ITR{
                typedef itr iterator_type;
            };
            template<typename citr, typename itr>
            struct IS_CONST_ITR<true, citr, itr>{
                typedef citr iterator_type;
            };            
            typedef typename IS_CONST_ITR<isConst, typename Container::const_iterator, typename Container::iterator>::iterator_type itr; 

            iterator_()
            :data_(), stepSize_(0){
            }

            iterator_(itr data, difference_type stepSize)
            :data_(data), stepSize_(stepSize){
            }

            iterator_(const iterator_<false>& src)
            :data_(src.getData()), stepSize_(src.getStepSize()){
            }

        [some more code ...] 

        protected:
            itr data_;
            difference_type stepSize_;
        };

        typedef iterator_<true> const_iterator;
        typedef iterator_<false> iterator;
};

Wouldn't it be easier to make `const_iterator` privately-convertible to `iterator`, and implement non-const `begin` in terms of const `begin`? — ildjarn, Apr 30 '12 at 22:24
Not if the conversion function is not publicly accessible... — ildjarn, Apr 30 '12 at 22:27
I guess you also need to make the container a friend of its iterator, that might work. — guinny, Apr 30 '12 at 22:29
You didn't show your `iterator`/`const_iterator` implementation, so I didn't want to get too specific. ;-] — ildjarn, Apr 30 '12 at 22:30
@chaiy - To me it looks like your elegant solution has more code than the original. How is that an improvement? — Bo Persson, May 01 '12 at 08:28

score 2 · Answer 1 · answered Apr 30 '12 at 22:36

2

what about:

class container{
    iterator begin(){
        return iterator(getStuff());
    }

    const_iterator begin() const{
        return const_iterator(getStuff());
    }

    private:
       Stuff getStuff() const  { [some code here] }
};

If you have common code.
Move it to a private method that does that job.

answered Apr 30 '12 at 22:36

Martin York

257,169
86
333
562

How is that different from the OP's `begin_impl` approach? – ildjarn Apr 30 '12 at 22:38
@ildjarn: I see no need to template it or make it static method. – Martin York Apr 30 '12 at 22:40
That doesn't seem sufficiently different to warrant an answer IMO. /shrug – ildjarn Apr 30 '12 at 22:41
@ildjarn: We can discuss it at the next Seattle meet up. But simple question dissevers simple answer that works. No point in over complicating things. – Martin York Apr 30 '12 at 22:50
@LokiAstari: there is still a problem with this answer. I think I have made a mistake in my code snippet. const_iterator and iterator of course takes different arguments in the constructor. as you see above, const takes const one while non-const takes non-const. – guinny Apr 30 '12 at 22:55
@chaiy: So. Both can still call a const method. `getStuff() const`. Obviously `[some code here]` is const otherwise you can not use it in the const version of `begin()`. If you want to get specific then you must show the code so we can see where it is actually different. – Martin York Apr 30 '12 at 22:56
@LokiAstari yes, I meant the Stuff for iterator is different than the Stuff for const_iterator. In my implementation iterator(std::vector::iterator) while const_iterator(std::vector::const_iterator) – guinny Apr 30 '12 at 22:58
1

@chaiy: Then show the correct code in the question. Half a question is useless. – Martin York Apr 30 '12 at 22:59
@chaiy: Seriously: You want something more elegant than you have? All the code dies is call begin(). Basically it is the same as my example except you have pushed getStuff into the internal container and renamed getStuff to begin(). Now I have to give you a -1 for a silly question. – Martin York Apr 30 '12 at 23:06
@LokiAstari Well, I just gave it a shot. Thanks for the effort. – guinny Apr 30 '12 at 23:08

score 1 · Accepted Answer · answered May 01 '12 at 19:53

Okay, apparently this remains the best answer:

template <T>
class container{
        template< typename I, typename C >
        static I begin_impl( C & c ){
            return I(data_.begin(), 1);
        }

        template< typename I, typename C >
        friend I container<T>::begin_impl( C & c );

        iterator begin(){
            return container<T>::template begin_impl< iterator >( *this );
        }

        const_iterator begin() const{
            return container<T>::template begin_impl< const_iterator >( *this );
        }
}

Avoiding duplicate code for begin() and end() functions in generic containers

2 Answers2