8

I have producers that take data A, produce data B and send it

    public interface Producer<T>{
        void produce(T data);
        void flush();
    }

    public class DataBaseProducer implements Producer<String>{
        List<String> producedData = new ArrayList<>();

        // create data
        public void produce(String data){
            producedData.add(transformData(data));
        }

       // send created data
        public void flush(){
           sendDataToDatabase(producedData);
        }
    }

   
   public class MessageProducer implements Producer<String>{
        public void produce(String data){
           String line =transformData(data)
           sendDataToMessageQueue(line);
        }

        public void flush(){
        }
   }

public static void main(String[] args) {
    // get producer
    Producer producer = getProducer(producerName)
    BufferedReader reader;
    try {
        reader = new BufferedReader(new FileReader(file..));
        String line = reader.readLine();
        while (line != null) {
            producer.produce(line)
            line = reader.readLine();
        }
        reader.close();
        producer.flush()
    } catch (IOException e) {
        e.printStackTrace();
    }
}

To demonstrate my question, imagine I have producers like above. One loads all data, and then bulk send it at once after it done, and second sends data right away (so it does not bulk send data, but whenever new data is created, it sends it right away)

Most of the producers will send the data after every data is loaded, but some of them will send them right away. If the producer sends data right away, than the flush() method remains empty. This however seems like bad practice and may violates some OOP principles. What is the correct way to implement this?

2
  • 4
    I would call this interface Consumer, not Producer. You are putting data into is, it consumes them. – jnovacho Jun 24 at 10:40
  • 1
    Those are consumers, not producers. The produce method should be called onProduce - or simply write. – Bergi Jun 24 at 15:00
24

This can be an entirely reasonable design, depending on what flush() is supposed to do.

If the flush() operation is more like a send() operation, then yes, silently sending this data before that method is called would be confusing.

But if the flush() operation merely ensures that the data is sent at the latest when this method is called, no problem: if the data is already sent, there's nothing more to do. An implementation that sends the data immediately and would have an empty flush() method would still conform to that interface!

Normally, a flush operation refers to the second kind: data may be buffered and might not have been sent before flush() is called, but the implementation is free to send the data earlier. For example, this is important to ensure that the buffer has a fixed maximum size.

Depending on your software, there is another aspect worth considering: when errors are raised. Sending the data immediately when the data was produced will also immediately raise IO-related errors. A user of this interface would have to expect such errors for both the produce() and flush() operations. If clear error handling is desired, it could make sense to mandate that the produce() method must buffer, and that a separate send() method must be called later. But at that point you're already forcing batch operations, and a sendBatch(List<T>) method would be the most general solution, no internal buffering needed.

1
8

There isn't anything necessarily wrong with this. The produce/flush base class interface can simply define that ambiguity as part of its contract.

We can clearly specify it in the documented requirements that produce is invoked, it is required to prepare the output in a buffer. The flush method passes on the buffered output, causing the buffer to be empty. Moreover, implementations of produce may pass on the output, as if they implicitly called flush before returning, in which case the implementation of flush can be a null operation.

The software which invokes this functionality has to always call flush, because produce is not required to call it.

The key ideas here are:

  1. The specification of an object behavior can be given in terms of an interface contract that spans multiple methods; in effect a protocol. The substitution principle is obeyed if every implementation of the object satisfies the protocol.

  2. From the point of view of the object system itself, it's enough that the object has a compatible flush method. That is to say, we would instantly have an OOP-level substitutability problem if the object whose flush is a no-op simply omitted implementing that function. Or, say, erring in the other direction: suppose some object has a flush but one that requires an argument, contrary to the base class specification which specifies a no-argument function. You've satisfied the most basic requirement for substituability by ensuring that all the derived objects have all the required methods, in a way that is call compatible.

It boils down to supporting all the required methods (basic substitutability at the OOP system level so the program builds and runs at all), and furthermore that they behave according to their documented protocol (rules of the interface contract spanning multiple calls to multiple methods). The protocol can specify deliberate latitudes about which method takes care of certain responsibilities, and those choices will be visible to a client which deviates from the protocol. (E.g. expecting that produce generates output instantly without calling flush, because it worked in one case with one kind of object.)

If we step back to the design though, it is probably best to avoid too many latitudes like this; there has to be a good reason for allowing certain flexibilities, such as that it is significantly easier for certain variations of the object to be implemented, or there is some useful efficiency gain.

If there is leeway about which methods do what, it creates challenges for unit testing. In this specific case, we cannot have a generic test of the flush function that works for objects of all types. We would like our test to call produce and then have our test case verify that no output has taken place, and then call flush, and check that the output was produced, and it would be nice if that worked the same way for every object. If it doesn't, then either we cannot have that kind of test, or the test has to "just know" that certain objects flush inside produce, and others don't, and kind of work outside of the official protocol. Once the test works outside of the protocol, then it's becoming coupled to the implementation details.

1

It's OK, see example from JDK source:

public interface Closeable extends AutoCloseable {

    /**
     * Closes this stream and releases any system resources associated
     * with it. If the stream is already closed then invoking this
     * method has no effect. (...)
     */
    public void close() throws IOException;
}

and implementation:

public class ByteArrayOutputStream extends OutputStream {

// (...)


    /**
     * Closing a {@code ByteArrayOutputStream} has no effect. The methods in
     * this class can be called after the stream has been closed without
     * generating an {@code IOException}.
     */
    public void close() throws IOException {
    }
}
2
0

There are already some excellent answers that take the specific problem, as a general rule I believe is usefull for the OP to ask himself the question: is the implementation of my interface fullfil the Liskov substitution principle?

The Liskov substitution principle apply to classes implementing interfaces as well

In others word, if the implementation of a specific class can break the code that is using the interface you are in trouble. In your case the behaviour of flush is entirely in line with the expectation of the interface user.

-1

There's two problems I would have with the above:

  1. There's a temporal coupling between the two methods that users have to understand.
  2. If the users don't read your documentation, or misunderstand what it says, they will misuse it. That probably includes you in a couple of months.

One idea to improve upon this design, is not trying to tell the user how to control the Producer, but give control to it. Like this:

public interface Producer<T> {
   void produce(Supplier<T> supplier);
} 

That would result in code like this:

reader = ...;
producer.produce(() -> reader.readLine());
reader.close();

So now the Producer can completely control what to do and when to do it, so it's a lot more difficult for the user to misuse. It can flush produced items regularly, knows when to clean up, etc.

There are other solutions. For example, if the flush() is only called once, you might just try to make it AutoClosable, (i.e. rename flush() to close()) so it can be used with try-with-resources constructs. That would be better then it stands now, but doesn't guarantee right usage.

3
  • Why the downvotes? – Right leg Jun 24 at 12:09
  • 2
    I'm not a downvoter, but this seems like a step backward - you can only flush output by terminating it? It's pretty normal in many cases (including filesystems and networking) to want to flush after a batch of work (and usually only the caller knows what constitutes a "batch"). – Toby Speight Jun 24 at 13:25
  • 1
    The Producer can flush as many times as needed in the above. The OP seems to suggest the producers control the flushing not the caller. – Robert Bräutigam Jun 24 at 18:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.