Type safe item access and events

[arneboe]

I still need to write some more unit-tests and some comments but it is generally done. Please read the diff and point out anything strange :)

[Rauldg]

Here are some comments:

1) This is done in multiple methods

if(vertex(frame) == null_vertex())
{
    throw UnknownFrameException(frame);
}
Do all the stuff 

wouldn't it be nicer to have a method for this?

bool validFrame(frame) ---> This triggers the UnknownFrameException

if validFrame(frame)
{
  Do all the stuff
}

2) getItemsInternal and in containItems

getItemsInternal What is this for? Does it create an exception if the assert is not met?

assert(begin != end); //if a list exists it should not be empty 

Does it work also if there is only one item of the type searched? Wouldn't in that case begin and end have same value?

containItems

return mapEntry != frame.items.end();

Same thing: End is not the last element, right?

3) In removeItem and removeItemFromFrame

removeItemFromFrame and removeItem have many common lines, couldn't that be improved with auxiliar methods?

In this code, what is different between begin and cbegin?

        std::vector<ItemBase::Ptr>& items = mapEntry->second;
        std::vector<ItemBase::Ptr>::const_iterator baseIterator = item.base();
        //HACK This is a workaround for gcc bug 57158.
        //     In C++11 the parameter type of vector::erase changed from iterator
        //     to const_iterator (which is exactly what we need), but  gcc has not
        //     yet implemented that change. 
        std::vector<ItemBase::Ptr>::iterator nonConstBaseIterator = items.begin() + (baseIterator - items.cbegin()); //vector iterator const cast hack

4) In GraphItemEventDispatcher

void notifyTreeEvent(const GraphEvent& event)

Why TreeEvent and not GraphEvent?

virtual void itemRemoved(const TypedItemRemovedEvent<T>& event) {}
//TODO itemRemoved

Is still TODO?

[Rauldg]

In BOOST_AUTO_TEST_CASE(complex_remove_transform_test) of test_transform_graph.cpp

Is this one missing? BOOST_CHECK_NO_THROW(tree.getFrame(b));

[arneboe]

1) Good point, I'll fix it.

2) asserts crash the program in debug mode. In release mode the assert is ignored. asserts are used to catch programming mistakes.

the begin != end works because end does not point to the last entry but behind the last entry.

3) I thought about that as well but they are already hard to read. Creating an auxiliary method would make it even harder to read because the method would not have any logical meaning.

In this code, what is different between begin and cbegin? cbegin() returns a const_iterator while begin() returns a normal iterator. I have to use cbegin() to be able to subtract from baseIterator which is const as well.

4) In GraphItemEventDispatcher Why TreeEvent and not GraphEvent? Good point, will fix that. It somehow survived the renaming from tree to graph