1

We have a desktop Swing application. It executes operations against a DB. It could be plain DML, it could be procedures. In both cases, it could result in an error. In that case, we need to display a dialog window to the user

I want to do it through a common reusable abstraction that I can easily mock in unit tests

public interface Notifier {
 // Notification is some data class: may contain status, summary, etc 
 void notify(Notification notification);
}
// some GUI panel
// I originally named it Report instead of Notification
Notification deleteNotification = service.delete(something);
// may display error dialog if it's an error
notifier.notify(deleteNotification);

But here's the problem. Our procedures return a DTO that, among other things, specifies what dialog we should show. And while the error/warn distinction poses no problem, the DTOs can also (rarely) mandate dialogs with the "Retry" option

And that changes everything

The Notifier abstraction doesn't care about (or requests) the user's response at all. It's a strictly one-way interaction. But because of those rare cases when further execution of the program depends on the user's choice, I now have to either

  1. Create two abstractions: one for handling errors of plain DML operations, another for handling procedure errors (because of those rare cases when user feedback is important)
  2. Or change the Notifier abstraction to make it return some UserChoice object (could be a enum)
public interface Notifier {
 UserChoice notify(Notification notification);
}
public enum UserChoice {
 OK, NO, RETRY;
}
// some GUI panel
UserChoice userChoice;
do {
 Notification procedureNotification = service.performSomeProcedure(someInputs);
 userChoice = notifier.notify(procedureNotification);
} while (userChoice == UserChoice.RETRY); 

The first option does not appeal to me at all

I like the second option more. Besides, I could use the same abstraction to request user confirmations ("Are you sure you want to delete this?")

Though, I'm not 100% sure about it too. Why? Because the return value won't be used 90% of the time. And if I split it into two methods, you may start to notice that it does two things: notifying the user and prompting them for feedback

public interface Notifier {
 void notify(Notification notification);
 UserChoice requestFeedback(Notification notification);
}

On the other hand, you can reframe the abstraction somewhat and make it responsible for interacting with the user generally. Then, it seems alright that it has one method for one-way interaction and another one for two-way interactions. I would have to come up with some generic name, and the purpose of that type would become more blurred (which is a downside)

public interface CommunicationChannel {
 void notify(Notification notification);
 UserChoice requestFeedback(Notification notification);
}

So what do you think? What is the right way to abstract that logic away?

asked Aug 18, 2024 at 8:00
12
  • Can you give us an example showing which layer implements this interface, which layer calls it and how it is used? Commented Aug 18, 2024 at 9:32
  • @DocBrown does my edit suffice? Commented Aug 18, 2024 at 11:11
  • Partially - this shows a GUI panel calling the notifier - but not how / where this interface is implemented? Is your notifier just a GUI component with a single implementation? Or are there multiple implementations? I also don't understand exactly what happens when the notification does not contain any error - simply nothing? Commented Aug 18, 2024 at 12:27
  • ... And how will the notifier be used when the user can return a "yes/no/retry"/cancel/something else" result? I think when a GUI panel does not know whether a user reaction might happen or not in a generic implementation, it has to be prepared for it, even when that is a rare case. Commented Aug 18, 2024 at 12:37
  • @DocBrown it doesn't matter how it's implemented, that's point. A prod implementation may go like, "Oh, it's a warn-level notification, and I'm set to track warn-level notifications and higher? Then, I'll take that notification's summary string and display it in a new warning dialog window". A test implementation will probably do nothing Commented Aug 18, 2024 at 13:43

2 Answers 2

2

Let me restate your problem, so I can describe my guess about where the core problem is located.

Your idea is to let the database layer return some object ("Notification") about the last operation's result, and to have a GUI component ("Notifier") which processes that object generically. Unfortunately, this approach reaches its limits in contexts where the result is not just "Ok" or "Error occured, try something else". A "retry" logic then may require a change in control flow in the non generic part of the GUI, and since your GUI always has to expect the DB to return a notification which requires a "Retry" logic, you may need to add the do ... while (userChoice == UserChoice.RETRY) ceremony in all kinds of places, even when it is unlikely that it is needed.

This, however, contradicts your idea of simplifying the GUI code by introducing the Notifier - instead of making the code less complicated, it becomes more complicated in several places. Note this problem will remain even when changing the interface from one method into two, hence I would stay with the one-method interface, which is simple and understandable as it is.

I think you have the following options:

  1. Live with it, and implement the usage of Notification always with do ... while (userChoice == UserChoice.RETRY).

    Advantage: it's dead simple. The logic whether a "retry" makes sense or not is kept inside the DB layer.

    Disadvantage: extra boilerplate code in more places than needed. Since the UI has no knowledge if a retry could be sensible, the do-while loop will have to be added in every place where the notififer is used.

  2. Live with it, but add do ... while (userChoice == UserChoice.RETRY) only in contexts where you know a "Retry" makes sense.

    Advantage: in the 90% of all cases where a retry does not make sense, the code is kept simple. If a required change of control flow looks different, it can be implemented easily.

    Disadvantage: this is a DRY violation - the knowledge that a retry can make sense (or may be required) is duplicated in the GUI layer as well in the DB layer. There is some risk the DB layer may request a retry, but the GUI forgets to support it!

  3. Keep the whole knowledge that a retry makes sense inside the database layer. Implement the retry logic generically inside some GUI component DBResponseProcessor, which calls the Notifier, like this

    class DBResponseProcessor,
    {
     void process(MyDbOperation dbOperation)
     {
     UserChoice userChoice;
     do {
     Notification procedureNotification = dbOperation();
     userChoice = notifier.notify(procedureNotification);
     } while (userChoice == UserChoice.RETRY); 
     }
    }
    

    Now, the user of this component has to pass a lambda expression for the db operation, like

     dbResponseProcessor.process(
     () -> service.performSomeProcedure(someInputs)
     );
    

    Advantage: calling code is short, the retry loop is implemented only once and does not leak into the calling code.

    Disadavantage: the reusable component is more complex, my above scetch is probably not sufficient and you need to work out the details. You may consider to merge "Notifier" and "DBResponseProcessor" into one component, for example. Moreover, the retry flow is fixed, other control flows may require additional generic code.

  4. Keep the whole knowledge that a retry makes sense inside the GUI layer. Will look similar to 3, but here it makes sense to have two entry points like process and processWithRetry

    class DBResponseProcessor,
    {
     void process(MyDbOperation dbOperation);
     void processWithRetry(MyDbOperation dbOperation);
    }
    

    Note, in this case, the processWithRetry does not need to return anything, since the retry logic is completely managed inside the component.

You will have to decide whether the 3rd or 4th option is worth the hassle, or if you can live with the more simpler options.

Let me add that in my experience, an immediate "retry" of a database operation makes only sense in very rare cases. Usually, a user needs to check the reason for a failed database operation, reestablish a connection first, fix something in the data, or restart the application. These are things they have to do separately first before a retry has a chance to succeed. So you should check if you really need the "retry" logic in a fully generic fashion, or whether it really makes sense to let the database layer decide alone if a retry should be done.

answered Aug 19, 2024 at 6:59
3
  • Thank you. So you think I should stick with UserChoice notify(Notification notification) (one method returning some choice option)? Commented Aug 19, 2024 at 7:55
  • @SergeyZolotarev: if I understood it correctly where your real problem is, I think the option of splitting the interface into two methods does not solve it. Commented Aug 19, 2024 at 7:59
  • @SergeyZolotarev: ... in the fully generic versions of my answer (3 + 4), the return code of "notify" will always evaluated, so a void variant won't make sense. Still, in #4, it can make sense to have two different entry points process and processWithRetry, as shown. Commented Aug 19, 2024 at 8:19
2

The notification and feedback request are two different things. I'd recommend to keep the separated - especially when 90% of time you need only one of them. Therefore:

/** Simple tool to notify user. */ 
public interface UserNotifier {
 void notify(Notification notification);
}

and

/** Tool for advanced interaction with the user. */
public interface UserFeedbackRequester {
 boolean askForConfirmation(Notification notification);
 RetryOrCancel askForRetry(Notification notification);
 // ... ask for something else
}

For convience, you can also declare (and probably implement) also:

public interface CompleteUserInteracter extends UserNotifier, UserFeedbackRequester {
 // nothing to be here
}

That way, whoever needs to just simply notify the user, will use the UserNotifier, and if someone needs to request a feedback, either use the UserFeedbackRequester or CompleteUserInteracter, if both will be nescessary:

public void foo(UserNotifier notifier) {
 doFoo();
 notifier.notify(new Notification("Foo done!");
}
public void foo(CompleteUserInteracter interacter) {
 try {
 doBar();
 interacter.notify(new Notification("Bar done!");
 } catch (SomeException e) {
 retryOrCancel = interacter.askForRetry(new Notification("Bar failed, try again?"); 
 if (retryOrCancel == RetryOrCancel.RETRY) {
 foo();
 }
 }
}
answered Aug 18, 2024 at 21:30
1
  • That does not fit the what the OP already told us: that the notification return by the DB layer carries already the information about which dialog to show and whether some user response has to be processed. Commented Aug 18, 2024 at 22:03

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.