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

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?

Solution

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