Anti Pattern: the need to call a function

published at 05.07.2013 18:11 by Jens Weller
Save to Instapaper Pocket

Every now and then I see this pattern coming around: the NEED to call a function. After you're done, you have to call cleanUpObjects() or endInsertItems() in some insane API. For me this is often an anti pattern, and sometimes its because the chosen design forces you to do so. If its up to you to design a system, maybe you should think about ways to avoid this, but that's a different topic. Today I just wanna rant about...

... wait that's no solution either. So, how to deal with this possible anti pattern, that comes every now and then to us? Not always one can avoid using such API, as for example you'll find it in Qt, and when using a mutex, well hello lock and unlock. Before I go to a possible solution, lets take a look at the problem:

void PersonModel::removePerson(int row)
{
    beginRemoveRows(QModelIndex(),row,row+1);
    std::vector::iterator it = mydata.begin();
    std::advance(it,row);
    mydata.erase(it);
    endRemoveRows();
}

 When implementing models in Qt, one has to deal with the methods of adding and removing Items from the model. The Modelimplementation in Qt wants you to call beginRemoveRows before starting this operation, and endRemoveRows after finishing. This code looks fine, and most likely never cause any problems. But what if it did? Lets say we call something that might throws, maybe our database or something within this code. Well, endRemoveRows is never called. If we would have this same pattern used with a mutex, calling lock and unlock at the beginning/end, well, our mutex is still locked after the exception. oops. Also, even if the code is right now fine, and will never ever cause a problem, next Monday maybe someone will edit this block and then boom. So, actually one needs to be able to deal with this.

RAII to the rescue

The mutex points at the right direction to solve this pattern. Because it is such a source of errors and possible (dead) locks, that this is solved in C++ for mutexes. Same goes for memory allocation, which would point to a first possible solution. A long time a go, some smart person came up with the idea, to use the scope of a code block, to do the locking and unlocking. An object is created on the stack of the function or method, which then calls lock in its constructor, and unlock in its destructor. As an exception causes a stack unwind, its guaranteed that even then unlock is called. The same mechanism is used by smart pointers, which use this to ensure that the memory is actually deleted. Sometimes you hear people say in teaching C++ "every new needs its delete", which can give the impression that this is 1:1, but actually its 1:n. There might be many places, where you would need to call delete, not all that obviously, so smart pointers take care of this. Most implementations of smart pointers also allow you, to hand over a custom deleter, where we could put in a lambda calling our function. One would need to maybe allocate a little extra in abuse... Em, thats not a good solution, so lets go on. So, what would be needed is some kind of smart function caller or so. Its a pity to implement this as a template:

template<class function>
class ensure_functioncall
{
    function f;
public:
    ensure_functioncall(function f):f(f){}
    ~ensure_functioncall(){f();}
};

Ok, its not perfect in the implementation maybe. But you could use a lambda to wrap the function call, and it should work. Further details for the implementation are left to the reader. That's NIH you say. Right. But it works, and its Friday! Even if its a trivial implementation (is it?), the problem of carrying it around, and using it every now and then is maybe not the solution one should look for solving. An actual implementation of such a ScopeGuard was presented by Alexandrescu at C++ and Beyond 2012. As it turns out, also boost has a solution for this, as a short discussion at twitter shows. boost::scope_exit is a possible solution to the problem, if you don't mind its ugly macros:

beginInsertRows(QModelIndex(),mydata.size(),mydata.size()+1);
BOOST_SCOPE_EXIT(this_){
this_->endInsertRows();
}BOOST_SCOPE_EXIT_END
mydata.push_back(std::move(person));

Obviously, this is the part in my model which adds data to it. As with the removal, I have to call beginInsertRows and NEED to call endInsertRows after this is done. BOOST_SCOPE_EXIT allows you to capture a couple of parameters, which you use inside the capturing block. When you refactor code, you will need to capture local variables and maybe edit the code for calling methods depending on this, as shown above. Of course you have to think about possible consequences, and how to deal with error handling. This code now ensures that endInsertRows is always called, even if this might fail. boost::scope_exit is a header only library and included in boost since 1.38. The code inside BOOST_SCOPE_EXIT will be executed on scope exit.

Last, scoped locking classes should be preferred. But for some mutex implementations you might not have a scoped locking mechanism, then this is quite handy. Or when dealing with APIs such as the other wise great Model/View concept of Qt. Alternative would be to wrap the code before the last function in a try/catch block, and handle at least catch(...).

Join the Meeting C++ patreon community!
This and other posts on Meeting C++ are enabled by my supporters on patreon!