How can I implements erase( iterator erase(const_iterator first, const_iterator last)) of vector in c++

Question

https://www.cplusplus.com/reference/vector/vector/erase/ I want to make erase() method of vector in c++

 iterator erase(const_iterator position)
    {
        theSize--;
        int index = position - begin();
        
        Object* newObj = new Object[theCapacity];
        for (int i = 0, j = 0; j <= index; ++j)
        
        {
                if (j != index)
                newObj[i++] = objects[j];
        }
        

        std::swap(objects, newObj);
        delete[] newObj;
        

        return &objects[index];
    }

First, I tried to make erase() and try to reuse it to make iterator erase(const_iterator first, const_iterator last)

   iterator erase(const_iterator first, const_iterator last)
    {
        int index = last - begin();
        for (auto it = first; it != last; ++it)
        {
            erase(it);
            
        }
        return objects;
       
    }

I don't know whether my approach was right or not. Since returned value is garbage value. I think my index was wrong. How can I improve my ** iterator erase(const_iterator position)**and How can I reuse my iterator erase(const_iterator position) to make iterator erase(const_iterator first, const_iterator last)?

v.erase(v.begin(),v.begin()+3)

input 541234

output -842150451-842150451

expected 1234

Answer 1

You don't need to reallocate the array; use move semantics instead. Even if you use the current implementation, you benefit from moving the values from the old array instead of copying them.

Furthermore it's more efficient for erase(const_iterator, const_iterator) to receive its own implementation.

Your actual problem here is the fact that you're invalidating the iterators by calling erase as noted by @UlrichEckhardt and even if you didn't reallocate the array, you'd need to call erase with first instead of it, since erasing the element at the current position of it shifts the remaining elements left by one resuling in the next element to be erased to be placed at the current position of it, not at the next position.

Here's a simplified vector implementation that should work:

template<class T>
class Vector
{
public:
    using iterator = T*;
    using const_iterator = T const*;

    Vector(std::initializer_list<T>&& list)
        : size(list.size()),
        objects(size == 0 ? nullptr : new T[size])
    {
        auto out = objects;
        for (auto& e : list)
        {
            *out = std::move(e);
            ++out;
        }
    }

    ~Vector()
    {
        delete[] objects;
    }

    iterator begin() noexcept
    {
        return objects;
    }

    iterator end() noexcept
    {
        return objects + size;
    }

    const_iterator cbegin() const noexcept
    {
        return objects;
    }

    const_iterator cend() const noexcept
    {
        return objects + size;
    }

    iterator erase(const_iterator pos)
    {
        auto const result = objects + std::distance(cbegin(), pos);
        auto const endIter = end();
        for (auto p = result; p != endIter;)
        {
            auto& lhs = *p;
            ++p;
            lhs = std::move(*p);
        }
        --size;
        return result;
    }

    iterator erase(const_iterator first, const_iterator last)
    {
        auto const result = objects + std::distance(cbegin(), first);
        if (first == last)
        {
            // empty delete sequence
            return result;
        }

        // shift the elements after last
        auto writeIter = result;
        auto readIter = objects + std::distance(cbegin(), last);

        for (auto const endIter = end(); readIter != endIter; ++writeIter, ++readIter)
        {
            *writeIter = std::move(*readIter);
        }
        // remove extra elements from the end
        size = std::distance(objects, writeIter);
        return result;
    }

private:
    size_t size;
    T* objects;
};

int main()
{
    {
        Vector<int> a = { 1, 2, 3, 4, 5 };

        auto iter = a.erase(a.cbegin() + 1, a.cend() - 1);
        std::cout << "element pointed to by returned value: " << *iter << '\n';
        for (auto i : a)
        {
            std::cout << i << '\n';
        }
    }

    {
        Vector<int> a = { 1, 2, 3, 4, 5 };

        auto iter = a.erase(a.cbegin() + 1);
        std::cout << "element pointed to by returned value: " << *iter << '\n';
        for (auto i : a)
        {
            std::cout << i << '\n';
        }
    }
}

Note: The loops in the member functions can be replaced by the overload of std::move taking iterators.