Split a method into two separate ones and reuse the same list across the invocations

Question

I have a method as follows (simplified for clarity and brevity):

public static List<Person> getPersons(String a, int b, DataSetX dataSetX, DataSetY dataSetY) {
        List<Person> persons = new ArrayList<>();
        
        if(dataSetX != null) {
            while (dataSetX.hasNext()) {
                // create an instance of person with very specific attributes set                
                persons.add(processX(dataSetX.next()));
            }
        }

        if(dataSetY != null) {
            while (dataSetY.hasNext()) {
               // create an instance of person with very specific attributes set               
                persons.add(processY(dataSetY.next()));
            }
        }
        return persons;        
    }

The method in reality is a bit more complicate than this (doing a bit more processing using also the a and b variables) but overall this is the method structure.

I was thinking to split this method into 2, one method for dealing with DataSetX and the other with DataSetY.
I was thinking to structure it as follows:

public static List<Person> getPersons(String a, DataSetX dataSetX, List<Person> persons) {
        if(dataSetX != null) {
            while (dataSetX.hasNext()) {
                // create an instance of person with very specific attributes set
                persons.add(processX(dataSetX.next()));
            }
        }
        return persons;
}

I would then call the methods as follows:

List<Person> persons = getPersons(a, dataSetX, new ArrayList<Person>());
getPersons(a, dataSetX, persons);    
// now I can use persons list with the result of both present

With this approach I reuse the same list and don't need to concat 2 different lists from 2 different methods if I just created the list inside the methods and returned.
On the other side it looks kind of weird and possibly error prone.

Is there a way to be able to split the function and avoid creating multiple lists and merging them (as I need 1 list in the end).
Is there some design pattern suited for this?

The first simple thing I would change, is change the name from getPersons to addPersons. Get seems that the method won't change the state in any parameter. — digao_mb, Oct 22 '22 at 19:24
@digao_mb: I omitted it because it is basically similar but only goes over `dataSetY`. I can add it if it causes confusion — Jim, Oct 22 '22 at 19:24

score 0 · Answer 1 · answered Oct 22 '22 at 20:27

In my view, this method is not simple and future user of this method will be surprised that this method adds data to List<Person> persons. Why? As method is named getPersons. In my view, people will be think that this method will just return data, not add new data to List<Person> persons.

So, it is better to be consistent with naming of method and do just one thing in method. So I would just read data in method:

public static List<Person> getPersons(String a, DataSetX dataSetX) {
    List<Person> persons = new ArrayList<>();
    if(dataSetX != null) {
        while (dataSetX.hasNext()) {
            // create an instance of person with very specific attributes set
            persons.add(processX(dataSetX.next()));
        }
    }
    return persons;
}

and the second method should look like the above method:

public static List<Person> getPersons(String a, DataSetY dataSetX) {
    List<Person> persons = new ArrayList<>();
    if(dataSetX != null) {
        while (dataSetX.hasNext()) {
            // create an instance of person with very specific attributes set
            persons.add(processX(dataSetX.next()));
        }
    }
    return persons;
}

In addition, this method can be placed in repository layer. Read more about Repository pattern.

And then you can create a collection and add these data from two methods. I am not Java guy, however, I think it can be done like this:

List<Person> overallPersons = new ArrayList<>();
Collections.addAll(list, getPersons(String a, DataSetX dataSetX));
Collections.addAll(list, getPersons(String a, DataSetY dataSetX));

Read more how to add multiple items to collection in Java

So basically you are saying create 2 lists and merge them in a 3rd. The original function though is more optimal in the sense that it just creates 1 instance of a list. This is what I was trying also to achieve — Jim, Oct 22 '22 at 20:32
@Jim We are writing code for people, not for computer. In addition, [Is premature optimization really the root of all evil](https://softwareengineering.stackexchange.com/questions/80084/is-premature-optimization-really-the-root-of-all-evil). It is better to have simple with good naming code. In addition, you can profile your code with one list and with three lists. — StepUp, Oct 22 '22 at 20:45
@Jim Code Good code should be readable, testable. And if it is possible then code should have minimum dependencies and parameters. You will simplify your life and future developers by writing simple code :) — StepUp, Oct 22 '22 at 21:09

score 0 · Answer 2 · answered Oct 22 '22 at 20:27

0

In case if your datasets can be transformed in Stream you can try something like this:

Stream<Object> streamX = dataSetX.stream().map(e -> processX(e));
Stream<Object> streamY = dataSetY.stream().map(e -> processY(e));
List<Person> persons= Stream.concat(streamX , streamY).collect(Collectors.toList());

answered Oct 22 '22 at 20:27

artiomi

383
2
7

This is what I wanted to avoid. The original function loops over 1 list. The approach you mention creates 2 separate lists and then concats then creating a 3rd one. I was trying to keep the advantage of just having 1 instance of a list – Jim Oct 22 '22 at 20:29

Split a method into two separate ones and reuse the same list across the invocations

2 Answers2