dbus-java icon indicating copy to clipboard operation
dbus-java copied to clipboard

Request for API improvement with adding and removing signal handlers.

Open brett-smith opened this issue 3 years ago • 6 comments

This is not a bug, just a request for comments on possible API improvements to adding and removing signal handlers.

For example, say I have a GUI app. In this app, one of the windows I open will add a set of signal handlers when the window opens, and remove them when the windows. The app has various different types of windows that might listen, or stop listening to, various signals.

The code to achieve this a bit clunky, especially when used with lambda syntax, and may require keeping references to multiple objects to be able to remove signals.

private DBusSigHandler<SomeDbusObject.MySignal> mySigHandler;
private SomeDbusObject myObject;

void setupSignals() {
    myObject = connection.getRemoteObject(BUS_NAME, ROOT_OBJECT_PATH, SomeDbusObject.class).
    mySigHandler = (e) -> {
        // Do stuff
    };
    connection.addSigHandler(SomeDbusObject.MySignal.class, myObject, mySigHandler);
}

void tearDownSignals() {
    connection.removeSigHandler(SomeDbusObject.MySignal.class, myObject, mySigHandler);
}

This is not terrible, but it can get a bit excessive when you have many signals.

At the very least, it may be useful to do 2 things to reduce the typing a bit.

  • Return the handler reference in addSigHandler()
  • Provide a new removeSigHandler() than only takes the handler instance.

So the above code becomes.

private DBusSigHandler<SomeDbusObject.MySignal> mySigHandler;

void setupSignals() {
   mySigHandler =  connection.addSigHandler(SomeDbusObject.MySignal.class, 
     connection.getRemoteObject(BUS_NAME, ROOT_OBJECT_PATH, SomeDbusObject.class,  (e) -> {
        // Do stuff
    });
}

void tearDownSignals() {
    connection.removeSigHandler(mySigHandler);
}

brett-smith avatar Jul 18 '22 12:07 brett-smith

Sounds good to me. Feel free to provide a PR for this feature.

hypfvieh avatar Jul 18 '22 15:07 hypfvieh

Return the handler reference in addSigHandler()

Provide a new removeSigHandler() than only takes the handler instance.

While that works, another approach is to return a Closeable implementation token. Such as:

final var token = connection.addSigHandler( ... );
token.close();

It's a bit more flexible because the code that performs the closing doesn't need a handle to the connection, only the token.

ghost avatar Jul 18 '22 16:07 ghost

Having a Closeable of some sort sounds good to me too. I'll get a PR together.

brett-smith avatar Jul 18 '22 18:07 brett-smith

The change has been merged, can we close this issue?

hypfvieh avatar Jul 22 '22 10:07 hypfvieh

Does any library usage documentation/example need to be updated as part of the PR?

ghost avatar Jul 22 '22 15:07 ghost

I don't think so. But if you want to add some more samples or documentation, that would be great

hypfvieh avatar Jul 22 '22 15:07 hypfvieh