Undo/Redo using lists of shared_ptr

Question

I'm trying to implement a drawing program sort of like paint. I have two std::lists which contain shared_ptrs to Shapes. One is the “Undo” linked list, the other is for “Redo.” Before I call reset on the Shape shared_ptr, add the shared_ptr to the Undo linked list via push_back.

LRESULT CDrawView::OnLButtonUp(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL& bHandled)
{
   int xPos= GET_X_LPARAM(lParam);
   int yPos = GET_Y_LPARAM(lParam);
   end.X = xPos;
   end.Y = yPos;
   m_shape->setEnd(xPos,yPos);
   m_shape->Draw(m_GraphicsImage);
   Undo.push_back(m_shape);
   RedrawWindow();
return 0;
}

When an Undo command is given, I grab the shared_ptr at the back of the Undo linked list and move it to the Redo linked list. Then, clear out the m_GraphicsImage to white and finally try toiterate through the Undo list, redrawing everything.

LRESULT CMainFrame::OnUndo(WORD /*wNotifyCode*/, WORD /*wID*/, HWND /*hWndCtl*/, BOOL& /*bHandled*/)
{
   m_view.Redo.push_back(m_view.Undo.back()); //gets the first element that was Undo
   m_view.m_GraphicsImage.Clear(255); //Clears the board

   for(std::list<std::shared_ptr<Shape>>::iterator it = m_view.Undo.end(); it!=m_view.Undo.begin() ; it--)
   {
     it->get()->Draw(m_view.m_GraphicsImage);
   }
return 0;
}

I keep getting list iterator not deferencable....i'm just trying to create a simmple undo and redo

Answer 1

It is illegal to dereference an end() iterator, which is what is occurring in the first iteration of this loop:

for(std::list<std::shared_ptr<Shape>>::iterator it = m_view.Undo.end();

From the list::end() reference page:

Returns an iterator to the element following the last element of the container. This element acts as a placeholder; attempting to access it results in undefined behavior.

Use reverse_iterators, rbegin() and rend(), if you wish to iterate backwards:

for (std::list<std::shared_ptr<Shape>>::reverse_iterator i(l.rbegin());
    i != l.rend();
    i++)
{
}

As you have c++11 features available (std::shared_ptr) you can use auto to deduce the iterator type instead of explicitly typing it:

for (auto i(l.rbegin()); i != l.rend(); i++)
{
}