1

I am working on a custom vector (String_vector) based off of a custom string class (String). I am having trouble with my copy assignment, and I think I am in some sort of loop that doesn't terminate. In my implementation I return *this as shown here, where I implement the copy-and-swap method.

String class:

class String{
    public:
    int len;
    char *str;

    String()
    :len(0), str(nullptr){}

    String(char const* S)
    :len(strlen(S)), str(new char[len +1]){
        assert(S != 0);
        strcpy(str, S);
    }

    ~String(){
        delete[]str;
    }
};

String_vector class:

class String_vector{
    public:
    String_vector(std::initializer_list<String>);
    String* base = nullptr;
    String* last = nullptr;
    String* limit = nullptr;

    String_vector(){};
    String_vector(const String_vector& v){
        reserve(v.size());
        base = last;
        last = uninitialized_copy(v.base, v.last, base);
    }

    //I think the issue is somewhere in here
    String_vector& operator=(const String_vector & v){
      String_vector p = v;
      swap(*this, p);
      return *this;
    }

  ~String_vector(){
    clear();
    deallocate(base);
  }

  void clear(){
    while(last > base){
      (--last)->~String();
    }
  }

  size_t size()const{
    return last - base;
  }
  void swap(String_vector & v1, String_vector & v2){
    std::swap(v1.base, v2.base);
    std::swap(v1.last, v2.limit);
    std::swap(v1.limit, v2.limit);
  }

  void reserve(std::size_t n){
    if(!base){
      base = allocate<String>(n);
      last = base;
      limit = n + base;
    }else{
      String* p = allocate<String>(n);
      String* q = p;
      for(String*i = base; i != last; ++i){
        new(q)String(*i);
        ++q;
      }
      for(String*i = base; (i=last); ++i){
        i ->~String();
      }
      deallocate<String>(base);
      base = p;
      last = q;
      limit = base + n;
    }
  }

    template<typename T>
inline T*
allocate(int n)
{
  return reinterpret_cast<T*>(::operator new(n * sizeof(T)));
}

template<typename T>
inline void
deallocate(T* p)
{
  ::operator delete(p);
}


template<typename T, typename... Args>
inline T*
construct(T* p, Args&&... args)
{
  return new (p) T(std::forward<Args>(args)...);
}

template<typename T>
inline void
destroy(T* p)
{
  p->~T();
}

template<typename T>
inline T*
uninitialized_copy(T const* first, T const* last, T* out)
{
  while (first != last) {
    construct(out, *first);
    ++out;
    ++first;
  }
  return out;
}

size_t capacity()const{
    return limit - base;
  }

void push_back(String const & s){
    if(!base){
      reserve(8);
    }else if(last == limit){
      reserve(2*capacity());
    }
    construct(last++, s);
  }

};

String_vector::String_vector(std::initializer_list<String> list)
  : base(), last(), limit()
{
  reserve(list.size());
  for (String const& s : list)
    push_back(s);
}

and main:

int main()
{
    String_vector v1 {"a", "b", "c"};
    String_vector v2;
    v2 = v1;
    return 0;
}

Thanks!

Community
  • 1
  • 1
iambicSam
  • 347
  • 1
  • 2
  • 11
  • Why are you using `reinterpret_cast`? – CinchBlue Apr 09 '16 at 00:02
  • And when you set a break point in your assignment operator, what do you see happening in the debugger? – kfsone Apr 09 '16 at 00:02
  • 1
    Your string violates the Rule of Three. https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming) – Mooing Duck Apr 09 '16 at 00:11
  • Also, `std::swap` is implemented using `operator=` (http://ideone.com/MtQU1P), so your temporary copy to `p` is simply a wasted operation. – kfsone Apr 09 '16 at 00:11
  • @kfsone: it's not wasted because he need a mutable copy. Plus, he's not using `std::swap` on the vector, only on primitives. Placement new is not a good suggestion for this case at all IMO. – Mooing Duck Apr 09 '16 at 00:13
  • @MooingDuck He only needs a mutable in-order to do `swap(*this, p);`. Placement new suggestion removed, though. – kfsone Apr 09 '16 at 00:15

0 Answers0