I had this post planned for a long time, but here it goes.
When we were developing the renderer some time ago, we saw that code was getting out of control because handling state changes was becoming hell, so we decided to rework it applying the State Pattern. Though Zeenix thinks it was my wife‘s idea, it was actually Iago‘s.
The other day I found a function that was sinning against that pattern. It was _update_playcount_cb
. When it was written, the call had been place in a central point where every media change going thru, but it was a bit ugly as it was using an if statement checking for the current state. This application of the state pattern is not completely canonical as we keep the current state but only because state classes are stateless and we want to keep them as Singleton’s, so the easiest way was keeping that variable to point the current state, but of course using it in an if statement is ugly.
Fortunately, I checked where it was needed to call that function and it saw it was in couple of places, so I reworked the code a bit and place the calls in the exact places where it was needed, so i didn’t need to make that function support the state pattern (in fact it would have been stupid).
But this state pattern still needs some finetuning. We’d need to remove the worker and increase the number of states and add substates to control for example the buffering and hide properly some GStreamer state changes that would make the code easier to understand and maintain.