Amarok/Proposals/ServicesReArchitecture
Services Re-Architecture
Discussion
A ML thread sparked by change to mp3-store service so it doesn't start with a modal dialog that breaks amarok startup. Matej had some good insights:
no Internet Service (in Amarok sense) should pop-up a dialog during first start, worse if it is modal no Internet Service should even be loaded (and thus sending any data) until necessary. They should be loaded when user first explicitly clicks on the store in Internet Services. Programatically, the services could be "registered" at startup - their object created. "Loading" a service could be calling its init() method or similar (I think that many services have a polish() method that's really abused right now). Before "loading", the services should have an opportunity to show a config dialog (preferably embedded in the view) that could request consent for sending data.
Proposal
I don't know much about services, but I've seen a lot of crap there.
First, there is a method, called polish() which is supposed (according to comment) to perform costly initialization. Actually, the services I've seen, just call it in the constructor.
Second, there is a <Servicename>Config class in every service, which does the same basic thing a wee bit differently.
So, my proposal:
split non-UI and UI initialization correctly, so that 1) no extra code is executed until it's needed; 2) no methods need to be called from constructor because they don't work otherwise, add a ServiceConfig or ServiceBaseConfig class, which does what all *Config do, the side-issues of asking users consent and everything can be implemented in Amarok itself. Proposed ServiceConfig class interface:
class ServiceConfig { public: ServiceConfig(QString name); template<class T> T get(QString variable, const T& default = T()); template<class T> void set(QString variable, const T& value); };
This way you can either use them in the service like this:
/* ... */ config.get<QString>("username"); config.get<int>("timeout");
Or extend ServiceConfig like this:
class CIAConfig: public ServiceConfig { public: CIAConfig(): ServiceConfig("cia") {} QString username(){ return get<QString>("username"); } int timeout(){ return get<int>("timeout"); } };
hades 15:04, 22 August 2012 (UTC)
Before running a service for the first time, Amarok can render some kind of "Are you sure you want to enable <servicename>?" widget. If the service requires some variables, this can either be provided (somehow) by its <Servicename>Config class, or just by returning a QWidget from a certain method.
hades 15:06, 22 August 2012 (UTC)