On Wed, Mar 08, 2006 at 04:28:30PM +0100, Abdelrazak Younes wrote: > #ifndef _RANDOM_ACESS_LIST_H > #define _RANDOM_ACESS_LIST_H
#ifndef RANDOM_ACESS_LIST_H #define RANDOM_ACESS_LIST_H Leading underscore + capital letter is reserved for the compiler implementation. Nothing we are supposed to use. > #include "debug.h" > > #include <boost/utility.hpp> > > #include <vector> > #include <list> > #include <algorithm> > > using std::endl; > > /// Random Access List. > /** > This templatized class provide a std::vector like interface to a standard > std::list > underneath. An important property is that it keeps the std::list::iterator > interface. Please restrict line length to 72 (+\epsilon) chars. > template <class T> > class RandomAccessList > { > protected: > typedef std::list<T> Container; > typedef typename Container::const_iterator base_cit; > typedef typename Container::iterator base_it; > > public: > > typedef T value_type; > typedef typename Container::difference_type difference_type; > typedef size_t size_type; > > /// > class iterator: public Container::iterator > { > public: > iterator() { > } > iterator(base_it const & it) { > Container::iterator::operator=(it); > } > iterator & operator+=(difference_type pos) { > std::advance(*this, pos); > return *this; > } > iterator operator+(difference_type pos) const { > return boost::next(*this,pos); > } > difference_type operator-(iterator it2) const { > return std::distance(it2, *this); > } It shouldn't be much of a difference, but the canonical way for such a 'symmetric' operation is to have it as free function outside the class. > }; > > /// > class const_iterator: public Container::const_iterator > { > public: > const_iterator() { > } > const_iterator(base_cit const & it) { > Container::const_iterator::operator=(it); > } > const_iterator(iterator const & it) { > > Container::const_iterator::operator=(static_cast<base_it>(it)); > } > const_iterator & operator+=(difference_type pos) { > std::advance(*this, pos); > return *this; > } > const_iterator operator+(difference_type pos) const { > return boost::next(*this, pos); > } > difference_type operator-(const_iterator it2) const { > return std::distance(it2, *this); > } > }; > > private: > /// Our container. > Container Container_; > /// Our vector of container iterator. > std::vector<iterator> ItVector_; I am not sure there are many precedence for capitalized member variables in LyX code. > > public: > /// > RandomAccessList() { > } > > /// Copy constructor. > RandomAccessList(RandomAccessList const & ext_list) { > const_iterator it_end = ext_list.end(); > for (const_iterator it = ext_list.begin(); it != it_end; ++it) > push_back(*it); > } > > /// Partial copy constructor. > /** > Copy "length" elements from ext_list beginning at pos. > */ > RandomAccessList(RandomAccessList const & ext_list, size_t pos, size_t > length) { > size_t end = pos+length; size_t const end = pos + length; > for (size_t i = pos; i != end; ++i) > push_back(ext_list[i]); > } > > /// Construct a new RandomAccessList by copying elements pointed from > it_start to it_end. > template<class It> > RandomAccessList(It it_start, It it_end) { Maybe adding 'reserve' is in order. > for (It it = it_start; it != it_end; ++it) > push_back(*it); > } > [...] 'swap' would be nice to have in the long run... > > /// Erases 'length' elements starting from pos. > /** > \todo This could be optimized a bit by rebuilding ItVector instead of > multiple deletion. > > Comments from Angus: use std::remove_if and vector<>::erase, passing the > same predicate to remove_if as is used by erase(size_t); > Refactor the other erase() member funtions to use this predicate > also? > std::remove_if doesn't actually remove anything from the vector. It > simply rearranges the > contents so that the stuff to be removed is all at the end, making it > trivial > (and cheap) to then used "itVector_.erase(it, itVector_.end());" to > resize the > vector. (it here is the iterator returned by std::remove_if). You just > need to > write the predicate that would actually flag up elements for removal. > */ > bool erase(size_t pos, size_t length) { > bool result=true; > size_t end = pos+length; Spacing > for (size_t i = pos; i != end; ++i) > result = result && erase(i); > > return result; > } > > /// Erases last element. > void pop_back() { > lyxerr[Debug::ACTION] << "pop_back from Container"<< endl; > Container_.pop_back(); > ItVector_.pop_back(); > lyxerr[Debug::ACTION] << "pop_back done"<< endl; > } > > /// Erases the element at position pos. > bool erase(size_t pos) { > > if (pos >= ItVector_.size()) { > lyxerr[Debug::ACTION] << "trying to erase element at " > << pos << " while size is "<< size() << endl; > // What happened here? > return false; > } > > if (pos == ItVector_.size()-1) { > pop_back(); > return true; > } > > lyxerr[Debug::ACTION] << "erasing element from Container at " > << pos << endl; > Container_.erase(ItVector_[pos]); > ItVector_.erase(ItVector_.begin()+pos); > lyxerr[Debug::ACTION] << "erasing done"<< endl; > return true; > } > > bool erase(iterator it) { > size_t pos = std::distance(Container_.begin(), > static_cast<base_it>(it)); > return erase(pos); > } > > bool erase(iterator start, iterator end) { > size_t startpos = std::distance(Container_.begin(), > static_cast<base_it>(start)); > size_t endpos = std::distance(Container_.begin(), > static_cast<base_it>(end)); > return erase(startpos, endpos-startpos); > } const + spacing. > > void push_back(value_type const & new_element) > { > lyxerr[Debug::ACTION] << "push_back new_element in Container"<< > endl; > iterator it = Container_.insert(Container_.end(), new_element); > ItVector_.push_back(it); > lyxerr[Debug::ACTION] << "push_back done"<< endl; > } > > /// Inserts the contents of an external container at position pos. > /** > The subsequent elements are shifted toward the end. > > \todo This could be optimized a bit by rebuilding ItVector instead of > multiple insertion. > */ > template<class SomeContainer> > bool insert(size_t pos, SomeContainer const & some_container) { > > bool result=true; > size_t i=pos; > typedef typename SomeContainer::const_iterator It; > It it_end = some_container.end(); > for (It it = some_container.begin(); it != it_end; ++it) { > result = result && insert(i, *it); > ++i; > } > return result; > } > > /// Inserts an external RandomAccessList at position pos. > /** > The subsequent elements are shifted toward the end. > > \todo This could be optimized a bit by rebuilding ItVector instead of > multiple insertion > and maybe using list::splice. > */ > bool insert(size_t pos, RandomAccessList const & ext_list, size_t > ext_pos, size_t length) { > > bool result=true; > size_t i=pos; > size_t ext_end = ext_pos+length; > for (size_t j = ext_pos; j != ext_end; ++j) { > result = result && insert(i, ext_list[j]); > ++i; > } > return result; > } > > /// Inserts external elements pointed by iterator it_start and it_end > at position pos. > /** > The subsequent elements are shifted toward the end. > \return the position after the last inserted element. > > \todo This could be optimized a bit by rebuilding ItVector instead of > multiple insertion > and maybe using list::splice. > */ > template<class It> > size_t insert(size_t pos, It it_start, It it_end) { > > bool result=true; > size_t i=pos; > for (It it = it_start; it != it_end; ++it) { > result = result && insert(i, *it); > ++i; > } > if (result) > return i; > > return size(); > } > > /// Inserts external elements pointed by iterator it_start and it_end > before element pointed by it. > /** > The subsequent elements are shifted toward the end. > \return the iterator after the last inserted element. > > \todo This could be optimized a bit by rebuilding ItVector instead of > multiple insertion > and maybe using list::splice. > */ > template<class It> > iterator insert(iterator it, It it_start, It it_end) { > > size_t pos = std::distance(Container_.begin(), > static_cast<base_it>(it)); > pos = insert(pos, it_start, it_end); > if (pos < size()) > return ItVector_[pos]; > > return Container_.end(); > } > > /// Inserts new_element at position pos. > /// The subsequent value_types are shifted toward the end. > bool insert(size_t pos, value_type const & new_element) { > > if (pos > ItVector_.size()) { > lyxerr[Debug::ACTION] << "trying to insert element at " > << pos << " while size is "<< size() << endl; > // What happened here? > return false; > } > > if (pos == ItVector_.size()) { > push_back(new_element); > return true; > } > > lyxerr[Debug::ACTION] << "inserting new_element in Container at > "<< pos << endl; > iterator it = Container_.insert(ItVector_[pos], new_element); > ItVector_.insert(ItVector_.begin()+pos, it); > lyxerr[Debug::ACTION] << "insertion done"<< endl; > return true; > } > > /// Inserts new_element before element pointed by iterator it. > /** > \return The iterator pointing to the newly inserted element > or Container_.end() if the insertion did not succeed. > */ > iterator insert(iterator it, value_type const & new_element) { > > size_t pos = std::distance(Container_.begin(), > static_cast<base_it>(it)); > if (insert(pos, new_element)) > return ItVector_[pos]; > > return Container_.end(); > } > > value_type & operator[](size_t pos) { > return at(pos); > } > > value_type const & operator[](size_t pos) const { > return at(pos); > } > > iterator operator()(size_t pos) { > return ItVector_[pos]; > } > > const_iterator operator()(size_t pos) const { > return ItVector_[pos]; > } > > RandomAccessList & operator=(RandomAccessList const & ext_list) { > assign(ext_list); > return *this; > } Could you move this up a bit closer to the constructors? Look pretty good in general. Andre'