Code review comment for lp:~bidossessi-sodonon/synapse-project/mpc-plugin

Revision history for this message
Alberto Aldegheri (albyrock87) wrote :

First of all, thank you for your contribution! :)

Some comments:
1)
MpcEngine class is instantiated each time I execute an action.
But MpcEngine doesn't have "attributes" that needs to be instantiated.
So I suggest to turn each method and attribute of MpcEnginge into a static one.
In example:
private static string bin = "mpc"; //this can be also "const"
public static void play_pause ()...

In this way you can call directly:
MpcEngine.play_pause ();

2) what's the meaning of
 >>search the database for a "type" and turn results to uris on the fly<< ?

3) Synapse's configuration capabilities will come asap (mhr3 and I are very busy in this moment)

« Back to merge proposal