26

I have a class, lets call it Calculator, with a method like this:

public double[] performCalculation(double[] someInData)

This method can generate a number of non fatal warnings (represented as an array of strings with warning names, such as InDataOutOfBounds). What is the most elegant and javaesque way to return these warnings to the caller? I see a few possibilities, none that I am particularly fond of:

  1. Return an instance of a custom class containing both the result of the calculation and the array of warings.
  2. Let performCalculation accept an additional argument String[] warnings that it populates with the warnings. This might be a no go, as it seems I can not resize the passed array, and there is no way of knowing in advance how many warnings there will be.
  3. Keep performCalculation as it is, but add a method String[] getWarnings() to Calculator, returning the warnings from the latest calculation.

EDIT: I am aware of this question. I would argue that this is not a duplicate, as the top answer to that question says that the answer is depending on the situation. This is a more specific case, that can get a more specific answer.

marked as duplicate by gnat, user22815, Community Apr 14 '16 at 13:47

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 5
    @gnat Top voted answer to that question says ...but it's difficult to comment without further information. This question is a more specific case, and I would argue therefor not a duplicate. – Anders Apr 13 '16 at 9:18
  • 5
    Not that I'm suggesting option 2 but why are you using a String[]. List<String> would give you what you need (ArrayList being the most common implementation). Arrays are sometimes useful (if for example fixed length is desired) but I reach for the list unless I have a good reason not to (or Collection if order isn't important) – Richard Tingle Apr 13 '16 at 9:46
  • Is there a particular reason you don't want to throw an exception for this? – Pharap Apr 13 '16 at 10:41
  • 1
    Yes. The user might want to actually perform the calculation despite the warnings, and should be able to do so. If I threw an exception inside performCalculation I could not return anything. – Anders Apr 13 '16 at 10:44
  • 1
    @Pharap Perhaps I used a bad name. It is not out of bounds as with array indexes. Rather it is outside the region of input that has the promised precision of the calculation. – Anders Apr 13 '16 at 11:31
50
  1. Yes. Do this. It's Java, do not fear classes. This is what classes are for.

  2. Really bad. It is slightly less bad with a Collection instead of an array, but you're still asking for ad-hoc ill-specified semantics that will never be used quite right.

  3. Can work if you are absolutely, positively certain that you will never, ever use Calculator except in a single-flow-of-control situation. Unfortunately, parallelizing things is one of the more common changes required after the fact.

  • 24
    I think 3 should be wholly discouraged since it adds temporal coupling for no good reason really. It's the type of "solution" that kiiind of "makes stuff work" in the same sense that a canvas held up by four sticks in the ground "kind of" is a house. – sara Apr 13 '16 at 9:50
  • 1
    @kai I agree unless the calculator was only ever spawned to perform one calculation. I find this unlikely since the OP said "latest" calculation. – corsiKa Apr 13 '16 at 17:05
  • To add to 1) - acquire and use a POJO generator - perhaps one using annotations like Lombok or Immutables - and you can choose this (almost always correct course of action) without any trouble at all. – davidbak Apr 13 '16 at 19:38
  • Similarly, but specific to this issue, try a Maybe or Either class, as commonly used in functional programming. Only problem is: The paradigm doesn't adapt nicely to Java which doesn't have the rich pattern matching syntax for overloaded functions and decomposing sum types that functional languages do. (Those two links given as examples and come without warranty; there are others available.) – davidbak Apr 13 '16 at 19:42
  • The unfortunate thing WRT 2 is that the Java developers themselves have engaged in it. docs.oracle.com/javase/8/docs/api/java/util/concurrent/atomic/… – JAB Apr 13 '16 at 20:17
26

Your option 1 is the best solution, I think. It's returning all results, including warnings, in a custom object. You're not maintaining state within the object performing your method call, and that means you don't have to worry about retaining and managing your state (e.g. in a threaded scenario, or even calling the method twice in succession and inadvertently losing results).

Don't be afraid of building custom objects for this sort of thing. As well as containing the fields, you can also add methods to this object to tie together this info and report on status e.g. returnResults.hasWarnings()).

You will likely find that more functionality falls into this return object. If I'm doing similar (e.g. in Scala), I often start by returning a tuple (sets of results tied together) and very quickly convert this into a new class with additional functionality)

  • 5
    Regarding that final paragraph - a great bonus is that you can add more information without breaking any consuming code. A lifesaver when you want to refactor later! – Gusdor Apr 13 '16 at 15:20
16

A fourth option, rather than passing in a String[] warnings (or more-flexibly a mutable List<String> warnings) as you suggest in #2, would be to pass in a callback listener or object that receives and displays the warnings. For fast calculations, this is very likely overkill, but may be a way to go if your calculation is long-running or otherwise needs to start showing asynchronous notifications. If you do use this pattern, you may want to supply a no-op listener implementation and an aggregating listener implementation for later inspection, and also consider making the listener nullable or optional (via overload).

public interface WarningListener {
  void warn(String warning);
  void showProgress(int percentage);
}

public Result performCalculation(Input input, WarningListener listener) { ... }

However, unless there were a specific reason to make the calculation call back early, I would go with option #1: In absence of structs a lightweight Java class is the best way to return multiple values as a single unit.

6

As others pointed out already, #1 is your best bet.

On the implementation side, you might want to take a look at the Notification Pattern

A notification is "An object that collects together information about errors and other information in the domain layer and communicates it to the presentation."

So, the essential idea is to bundle errors (or warnings, if you want) in a single object.

2

If the caller needs to be able to react to the warnings, the preferred way is your first proposal, that is return a custom type (as others have already pointed out).

However, most of the times, the caller is not interested in warnings. For these cases, such warnings would usually be sent to a Logger (see for example slf4j).

public double[] performCalculation(double[] someInData) {
    // ...
    if (dataOutOfRange) {
        LOGGER.warn("Input data {} is out of range!", data);
    }
    // ...
    return result;
}

Another way would be to let the caller pass in some kind of listener, using Java 8 lambda expressions.

public double[] performCalculation(double[] someInData, Consumer<Warning> warningConsumer) {
    // ...
    if (dataOutOfRange) {
        warningConsumer.accept(new Warning(WarningType.OUT_OF_RANGE, value));
    }
    // ...
    return result;
}
1

I think option 1 is best, but in my opinion there's a better way to do it than creating a custom Results class.

Simply use a 2Tuple, as some others have mentioned, of the result and the list of warnings.

public Tuple2<double[],String[]> performCalculation(double[] someInData){
    ...
}

Here is an example of how to implement a tuple

I believe this is better because in general classes should be reusable. A results class you use once to get these results isn't. But a Tuple is general, and can be reused in many instances.

  • 1
    Consider though the fact that an object makes the difference between the result and the warnings and result much more clear. The consumer has no way without prior knowledge to know which element of the tuple is the results and the warnings. Classes, as mentioned in another answer, also allow you to extend the functionality later if needed without breaking consumer code. The class is basically behaving as a struct. Since Java has no such construct, you just use classes - there's no reason a class needs to be reusable and this is a great case for that. – The6P4C Apr 14 '16 at 7:18
  • That's true, however I think this is a case where some documentation is all that's needed to clear up the ambiguity, and you save yourself from having to write more classes, which in my opinion, is a good thing. – Ryan Stull Apr 20 '16 at 14:39
  • Can you tell me why you think that writing more classes is a good thing? And is having documentation really an excuse for bad design, i.e. not creating a separate results class? – The6P4C Apr 21 '16 at 7:39
1

I've used a different approach in the past. In my scenario I had a large number of methods that could generate warnings. I created a Context class, which contains various things, including a list of warnings. It also has a static method getCurrentContext().

So, any time I need to generate a warning, I can do:

Context.getCurrentContext().addWarning("Email address is longer than 64 characters; truncating");

The benefit of this arrangement is that a warning can be created anywhere.

The main difficulty is implementing getCurrentContext. My application was a web application (a REST/JSON service), where each request is handled in a separate thread. At the start of a request, an empty context is created and stored based on the thread ID. getCurrentContext uses the current thread ID to get the Context. At the end of the request, when the output JSON is constructed, any warnings are included in the JSON.

In your scenario, if performCalculation is the only method that can cause warnings, then this probably isn't the solution for you.

I'd be interested in feedback on this general approach. It works very well for this particular app - but it does feel a little hacky.

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