1

I have a general design question. Suppose I have a List:

List<String> list = new ArrayList<>();

list.add("Str 1");
list.add("Str 2");
list.add("Str 3");

I need to process the elements of that list in a loop. There are at least 3 options that I can think of.

Passing only the list the perform the operations:

public List<String> processListInLoop(List<String> list) {
    for (int i = 0; i < list.size(); i++) {
        // do something with the element
        // list.get(index).
    }
    return list;
}

Passing the list with the index of an element:

public List<String> processListWithIndex(List<String> list, int index) {
    // do something with the element
    // list.get(index).

    return list;
}

Passing the element directly:

public String processElement(String str) {
    // do something with the element
    // str.

    return str;
}

The methods would be called like this:

// option 1
processListInLoop(list);

// option 2
for (int i = 0; i < list.size(); i++) {
    processListWithIndex(list, i);
}

// option 3
for (int i = 0; i < list.size(); i++) {
    list.add(processElement(list.get(i)));
}

Is there a rule about the granularity of a method? My guess would be to use option 1, as that method encapsulates most of the logic.

gaout5
  • 41
  • 2
  • Do you have a specific use case in mind, or is your question more general? You could make a case for all these options depending on your context. – Vincent Savard Aug 03 '21 at 14:22
  • Your examples are confusing because they do not perform the same logic. Neither of them seems better than doing it all inline because the iteration is one easy to grasp thing that is now broken up, not making things easier on the mind. – Martin Maat Aug 03 '21 at 18:34
  • Apply design principles strategically. If the typical usage within your application is to process collection of items, or if it is advantageous to keep everything in lists for some reason, or you have a specialized traversal logic, but want to pass in a lambda for the actual processing, go with (1). If not, and if there are a lot of cases where you process an isolated object, go with (3). Option (2) is doesn't really buy you anything, unless you need that index for something within the body of the function. – Filip Milovanović Aug 03 '21 at 19:49

2 Answers2

3

My guess would be to use option 1, as that method encapsulates most of the logic.

By that logic the only method you need is main(). Bleh.

There is no one good answer to your method granularity question because there no one good granularity to rule them all. In fact I'd argue that a good design might just put option 3 inside option 1. Will need better names first. And why are you returning the list if you're mutating it?

Good functions come in many granularities. Many levels of abstractions. Generally, shorter is better. But that doesn't mean there's any rule about putting looping inside or outside of a function.

There is value in being able to separate looping logic and the operation to be performed on the elements of the list. Which is why we've invented ways to keep them separate: map(), reduce(), foreach(), and other such methods that apply a lambda. There are also specialty functions like Arrays.fill() that keep you from looking at multiple semicolons on one line. There's also old school techniques like recursion that can simplify some problems (while sneaking in other problems).

Bugs love to hide in looping logic. Don't begrudge me the functions I need to make it easy to read.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
2

The general rule I use is:

Create a function such that it can have a meaningful name.

So I would choose the third method processElement (but with a different name) because:

  • It has the smallest input requirements;
  • How the method is supposed used has no impact on the implementation of the method;
  • It can be reused;
  • It will contain only small bit of business logic that can have a meaningful name.

Having small methods allows reuse and composition into higher-level functions.

Allow me to explain with a slightly expanded example:

// Names in a list
List<String> names = Arrays.asList("Abe Bloom", "Cedric Diggory");
List<String> firstLetters = names
  .stream()
  .map(::firstLetter)
  .collect(Collectors.toList());

// Names in other code
String name = person.getName();
String personFirstLetter = firstLetter(name); // re-use of the same function

/// --- ///

public String firstLetter(String name) {
  return name != null && name.length() > 0 ? name.charAt(0) : null;
}

Notice that I named the list names instead of list, and the method firstLetter instead of processElement. These names reflect the content of the list/method.

Hidde
  • 170
  • 7