ef7676e3   
   From: ulrich.eckhardt@dominolaser.com   
      
   Am 10.07.2012 01:02, schrieb Jimmy H.:   
   >   PreallocatedArray(PreallocatedArray  &other) :   
   >       valid_size(other.valid_size) {   
   >     for (size_t s(0); s < valid_size; ++s) {   
   >       new (&operator [](s)) T(other[s]);   
   >     }   
   >   }   
      
   The array to copy from should be const. Further, imagine one of these   
   copy constructors throws. You then don't destroy any objects, not even   
   those that were already constructed.   
      
      
   >   T &operator [] (size_t pos) {   
   >     return ((T *)memory)[pos];   
   >   }   
   >   const T &operator [] (size_t pos) const {   
   >     return ((T *)memory)[pos];   
   >   }   
      
      
   I personally dislike the C-style casts, although it requires two casts   
   doing it the C++ way. Looking at the const operator[], you also cast   
   away the const there, which is surely not intentional! Instead, I'd   
   introduce a single function "T* data()" for that. Then I'd add a "T   
   const* data() const" overload. Inside those two, I could live with ugly   
   code. However, everywhere else that you access the array, only those two   
   functions should be used.   
      
      
   >   T *begin()   
      
   Using a pointer as iterator works in most places, but not always. I'd   
   use Boost.Iterator to construct the missing parts on top of the pointer.   
      
   >   const T *cbegin() {   
   >     return &operator [](0);   
   >   }   
   >   const T *cend() {   
   >     return &operator [](valid_size);   
   >   }   
      
   Those two should not have different names, instead they should be   
   overloaded and declared const.   
      
      
   >   bool empty() [...]   
   >   size_t size() [...]   
   >   size_t max_size() [...]   
      
   These should all be const, too.   
      
      
   >   void swap(PreallocatedArray &other) {   
   >     using std::swap;   
   >     for (size_t s(0); s < N; ++s) {   
   >       if (s < other.valid_size && s < valid_size) {   
   >         std::swap(operator[](s), other[s]);   
   >       } else if (s < other.valid_size && s >= valid_size) {   
   >         new (&operator[](s)) T(std::move(other[s]));   
   >         (&other[s])->~T();   
   >       } else if (s >= other.valid_size && s < valid_size) {   
   >         new (&other[s]) T(std::move(operator[](s)));   
   >         (&operator[](s))->~T();   
   >       } else {   
   >         break;   
   >       }   
   >     }   
   >     swap(valid_size, other.valid_size);   
   >   }   
      
   The "using std::swap" is good, but then using "std::swap" explicitly in   
   the body isn't. I guess the same applies to "std::move". This could also   
   check for self-swapping.   
      
   >   PreallocatedArray &operator =(const PreallocatedArray &other)   
   >   {   
   >     for(size_t s(0); s < N; ++s) {   
   >       if (s < valid_size && s < other.valid_size) {   
   >         operator[](s) = other[s];   
   >       } else if (s >= valid_size && s < other.valid_size) {   
   >         new (&operator[](s)) T(other[s]);   
   >       } else if (s < valid_size && s >= other.valid_size) {   
   >         (&operator[](s))->~T();   
   >       } else {   
   >         break;   
   >       }   
   >     }   
   >     valid_size = other.valid_size;   
   >   }   
      
   Here, too, you need to keep track of the current size to correctly   
   handle exceptions in between. Also, the array will be half-assigned if   
   there is an exception, so I'd just create a temporary copy and then use   
   swapping.   
      
      
   Another thing I would suggest is to use assert() to check ranges etc.   
   The precondition of operator[] is that the index is valid. Verifying   
   this precondition inside the function helps a lot finding errors.   
      
      
   Good luck!   
      
   Uli   
      
      
   --   
         [ See http://www.gotw.ca/resources/clcm.htm for info about ]   
         [ comp.lang.c++.moderated.    First time posters: Do this! ]   
      
   --- SoupGate-Win32 v1.05   
    * Origin: you cannot sedate... all the things you hate (1:229/2)