Reusable Implementation of the Publish-Subscribe Pattern

I've finished an implementation of what I believe the Publish-Subscribe pattern is for practice and possible use in personal projects even though this implementation is fairly similar to the Observer/Observable classes of the Java API.

I'm looking for a review of my code-style, JavaDoc usage, API usage, implementation of the pattern, tests, and just about anything else you can think of.

Radio:

```
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Set;

public class Radio {
/** A {@link HashMap} of events mapped to listening {@link Receiver Receivers}. */
private final HashMap>> receivers = new HashMap<>();

/**
* Transmits an event without data.
*
* @param event
* The event to transmit.
*/
public final void transmit(final String event) {
if (event == null || event.isEmpty()) {
throw new NullPointerException("A transmitted event cannot be null or empty.");
}

transmit(event, null);
}

/**
* Transmits an event with data.
*
* @param event
* The event whose receivers are to be transmitted to.
*
* @param data
* The data to transmit to the {@link Receiver Receivers}.
*/
public final void transmit(final String event, final Data data) {
if (event == null || event.isEmpty()) {
throw new NullPointerException("The event cannot be null or empty.");
}

final Set> receivers = this.receivers.get(event);

if (receivers != null) {
receivers.forEach(receiver -> receiver.receive(event, data));
}
}

/**
* Adds a receiver to an event.
*
* @param event
* The event to add a receiver to.
*
* @param receiver
* The receiver to add.
*/
public final void addReceiver(final String event, final Receiver receiver) {
if (event == null || event.isEmpty()) {
throw new NullPointerException("The

Receiver

-
Data is a placeholder denoting a generic type. In Java, generics are pointed out with a single-letter literal (most often, T, E or U). The choice of Data is a bit misleading: one can easily understand that there is a type called Data, but indeed it is generic. Use a single letter name istead.

-
final keywords with arguments in interface method declarations do not have much sense: the implementor may declare them as non final, it's up to him to decide. So you can avoid extra verbosity and declare the method as void receive(String event, T data);.

Radio

-
Same remark about the generic type: it's better to use a single-letter literal.

-
The receivers field should be typed as Map (interface) instead of HashMap (implementation). Prefer interfaces to implementations where possible, especially for collections.

-
There are possible name clashes in the calls final Set> receivers = this.receivers.get(event);. Naming a local variable as a field of the parent object is not a crime, but is prone to errors and bugs.

-
if (receivers.containsKey(event) == false) is the same thing as if (!receivers.containsKey(event)), but consider using the dedicated receivers.putIfAbsent instead of calling containsKey.

Args Validation

It is not optimal for many of the methods:

-
there are code duplications. The checks should be extracted to a method like ensureStringArgNotNullNotEmpty(String arg, String argName), which should be able to produce an exception message with the name of the argument.

-
event.isEmpty() does not correspond to the meaning of the thrown exception. event is not null here, so why NullPointerException ?

I think that IllegalArgumentException is more appropriate for these validation checks.

If you still want to remain on NullPointerException, consider using one-liners with the standard Objects.requireNonNull call.

RadioTest

All the tests are clean, nicely coded and respect the principle of one assertion per test case.

But

• They are all poorly named.

Just compare the current naming:

transmitA();
transmitB();

with alternatives:

nullDataReturnedWhenTransmittingEventOnly();
nullDataReturnedWhenTransmittingEventWithNullData();

If something breaks, you'll easily find where the problem is, instead of decoding the meaning of A or B.

• The tests methods named transmit* make assertions about Receiver instances. Shouldn't they be placed in ReceiverJUnitTest cases ?

StackExchange Code Review Q#151495, answer score: 2