Is it a good practice to use the return of a function where the returned value is an object supplied as an argument?

Question

I just wanted to ask if this is a good java practice or there is a better way (official way) to do the same thing.

First of all, I need to update some hashmap information:

Map<Date, Object> rows = new HashMap<Date, Object>();

This is an object for an excel row, for each date (i.e. october first, october second, etc...) and an object containing this row's information.

So, in order to get this information, I have some methods such as:

rows = performQuery1(rows, foo, bar, initDate, endDate);
rows = performQuery2(rows, someDAO, foo);

And...

private HashMap<Date, Object> performQuery1(rows, foo, bar, Date, Date) {
  // Some code that adds or removes elements from the hashmap "rows"
  rows.put(date1, o1);

  //Then return the same object
  return rows;
}

So my question is: Is this a good java practice?

rows = performQuery1(rows, foo, bar, initDate, endDate);
rows = performQuery2(rows, someDAO, foo);

or not?

Answer 1

The question is indeed very broad, or, focussing on the "best practices" part, possibly opinion based - but not primarily opinion based, since there is a valid argument for a pattern like this.

Often you have a method that obtains data somewhere, and is supposed to put it into a target data structure (maybe a collection, or a map in your case).

There are several options for the signature of such a method (roughly in the line of your example, but this pattern can be generalized).

The first one could be

/**
 * Computes ... something, and returns the result as a map
 */
Map<Date, Result> execute(Date d0, Date d1) { ... }

The second one could be

/**
 * Computes ... something, and places the results into the
 * given map
 */
void execute(Date d0, Date d1, Map<Date, Result> results)  { ... }

---

However, for maximum flexibility, I often refer to the third option (which is the one that you actually asked about):

/**
 * Computes ... something, and places the results into the
 * given map, which is then returned. If the given map is
 * null, then a new map will be created and returned.
 */
Map<Date, Result> execute(
    Date d0, Date d1, Map<Date, Result> results)  { ... }

This has several advantages:

• You conveniently let the call create a new map:

  Map<Date, Result> results = execute(d0, d1, null);
  

• You can determine the implementation of the target data structure. If you always returned a new map, then there would be no way to choose between a HashMap or a LinkedHashMap, for example. Passing the target data structure to the method allows you to call

  Map<Date, Result> results = 
      execute(d0, d1, new HashMap<Date, Result>());
  

  or

  Map<Date, Result> results = 
      execute(d0, d1, new LinkedHashMap<Date, Result>());
  

  respectively

• You don't have to create a new map for each call. For example, you could create a sequence of calls

  Map<Date, Result> results = new HashMap<Date, Result>();
  execute(d0, d1, results);
  execute(d2, d3, results);
  

  accumulating the results in the given map

---

The power of this method may become even more obvious when considering that it can trivially emulate both alternatives:

class DB {

    // The private method that can emulate both public methods:
    private Map<Date, Result> executeImpl(
        Date d0, Date d1, Map<Date, Result> results);

    // The implementation that returns a new map
    public Map<Date, Result> execute(Date d0, Date d1) {
        return executeImpl(d0, d1, null);
    }

    // The implementation that fills a given map
    public void execute(Date d0, Date d1, Map<Date, Result> results) {
        executeImpl(d0, d1, results);
    }

}

---

An aside: A similar pattern is also used in some places of the Java SDK. For example, in a different application case: BufferedImageOp#filter:

BufferedImage filter(BufferedImage src, BufferedImage dest)

... If the destination image is null, a BufferedImage with an appropriate ColorModel is created.

Returns: The filtered BufferedImage

Answer 2

The heart of the question boils down to Is Java "pass-by-reference" or "pass-by-value"?

Moreover, you should either return a newly created object or modify objects referenced by parameters, but not both. There are some useful exceptions like Method chaining, but that does not seem to be the case here.

When you return data from your method, a programmer will expect that the data is a newly created object, while the objects referenced through parameters are left unchanged. Having the method return void (or a success code) hints the programmer that the method modifies an object referenced by a parameter instead of returning a new object and makes your code easier to read.