Code review comment for lp:~voldyman/granite/contractor-wid-dep-new-Contractor

Wow, fantastic work here!

Since overall this is very cohesive and well done, I'll just focus on a couple of details to make it even better IMHO:

(1) Why isn't ensure() private?
(2) Make sure the interface ContractorDBus is internal to Granite.
(3) The names of execute_* () suggest URIs, but in the parameters they are named paths. What do these really expect?

(1) and (2) are easily fixable.

Regarding (3), since this is a wrapper to the DBus API, I could suggest receiving GLib.File objects instead, and calling get_path() or get_uri() on them depending on what Contractor is expecting. Make sure you rename the methods accordingly (..._with_files).

Regarding the API, it would be nice to have something like:
    Granite.Services.Contract contract;
    File file;
    // get some instances from somewhere...
    contract.execute_with_file (file);

Instead of:
    string contract_id;
    string file_uri;
    // get those strings from somewhere...
    Granite.Services.Contractor.execute_with_uri (contract_id, uri);

We'd have something like:

public interface Granite.Services.Contract : Object {
    public abstract void execute_with_file (File file);
    public abstract void execute_with_files (File[] files);
}

public class Granite.Services.Contractor : Object {
    private struct GenericContract { ... }

    // Private implementation of Contract
    private class ContractorContract : Object, Contract {
        private GenericContract data;
        private Con

        public ContractorContract (GenericContract data) {
            this.data = data;
        }

        public void execute_with_file (File file) {
            // Yes, we have access to the parent class's private static methods
            execute_with_uri (data.id, file.get_uri ());
        }

        [...]
    }

    [...]

    // Of course, this method could be removed and its contents may be moved to
    // Contract.execute_with_file() directly.
    private static void execute_with_uri (string id, string uri);

    [...]
}

Finally, please fix the coding style issues. A quick read through the code will show what I mean. Of course there are only a couple of them :)