0
List<String> doSomething(String input){
  if(input == null){
     return Collections.emptyList();
  }

  List<String> lst = getListfromSomewhereElse(input)
  if(lst.isEmpty(){
     return Collections.emptyList(); //Approach 1
     // return lst;  //Return same empty list
   }
   // do some more processing on lst
   return lst;
}

I prefer approach 1, coz its more readable and explicit. What is better approch 1 or 2?

Question is if the list is empty should i return same list or explicitly create new empty list and return

8
  • approach 1 is better in my opinion. Commented Mar 13, 2018 at 4:24
  • 10
    why don't just return getListfromSomewhereElse(input) whether it is empty or not? Commented Mar 13, 2018 at 4:25
  • 2
    Better to satisfy what requirements? Both approaches are valid in different circumstances, and there's plenty of overlap where they are essentially equivalent. Your question is meaningless. Commented Mar 13, 2018 at 4:25
  • if (input != null) return getListFromSomewhereElse(input); return Collections.emptyList(); Commented Mar 13, 2018 at 4:26
  • 1
    If the caller expects to be able to add to the returned list, Collections.emptyList() will prevent that, as it is immutable, where as the empty list return from elsewhere could be mutable. Commented Mar 13, 2018 at 4:52

3 Answers 3

1

Collections.emptyList() return one constant member of Collections, so it takes no excessive time (can be optimized by JIT) and memory.

On the other side return of getListfromSomewhereElse possibly locks empty list returned from other code. Potentially you can get any list class and potentially it can take a bit of memory. Generally it's not a problem, as this method is also derived, reviewed and tested by your own team, but who knows what happens in outer libraries?

For example, getListfromSomewhereElse can read really large file into memory and then remove all elements from it. So, empty list will hold thousands elements capacity unless you/them know its structure and get rid of excessive capacity. Approach 1 will simply overcome this by usage of already existing constant list.

As a side note, if you process list elements in java 8 stream style, you naturally get new list with .collect(Collectors.toList()) step. But JDK developers don't force emptyList in this case.

So, unless you are sure in getListfromSomewhereElse, you better return Collections.emptyList() (or new ArrayList() or whatever list type you return by method contract).

Sign up to request clarification or add additional context in comments.

Comments

0

I would prefer

List<String> doSomething(String input) {
    List<String> list = new ArrayList<String>();

    if (input != null) {
        List<String> listFromSomewhereElse = getListfromSomewhereElse(input);
        list.addAll(listFromSomewhereElse);
    }

    return list;
}

Keep in mind that Collections.emptyList() is unmodifiable. Depending on the result of getListFromSomewhereElse a client of doSomething might be confused that it can sometimes modify the list it gets and under some other situation it throws an UnsupportedOperationException. E.g.

List<String> list = someClass.doSomething(null);
list.add("A");

will throw UnsupportedOperationException

while

List<String> list = someClass.doSomething("B");
list.add("A");

might work depending on the result of getListFromSomewhereElse

Comments

0

It's very seldom necessary to do (pseudocode):

if(input list is empty) {
    return an empty list
} else {
    map each entry in input list to output list
}

... because every mainstream way of mapping an input list to an output list produces an empty list "automatically" for an empty input. For example:

List<String> input = Collections.emptyList();
List<String> output = new ArrayList<>();
for(String entry : input) {
    output.add(entry.toLowerCase());
}
return output;

... will return an empty list. To treat an empty list as a special case makes for wasted code, and less expressive code.

Likewise, the modern Java approach of using Streams does the same:

List<String> output = input.stream()
   .map( s -> s.toLowerCase())
   .collect(Collectors.toList());

... will create an empty List in output, with no "special" handling for an empty input.


Collections.emptyList() returns a class that specifically implements an immutable, empty list. It has a very simple implementation, for example its size() is just return 0;.

But this means your caller won't be able to modify the returned list -- only if it's empty. Although immutability is a good thing, it's inconsistent to sometimes return a immutable list and other times not, and this could result in errors you detect late. If you want to enforce immutability, to it always by wrapping the response in Collections.unmodifiableList(), or using an immutable list from a library like Guava.


You also test whether the input is null. Consider whether this is necessary. Who's going to be calling the method? If it's just you, then don't do that! If you know you're not going to do it, your code needn't check for it.

If it's a public API for other programmers, you might need to handle nulls gratefully, but in many cases it's entirely appropriate to document that the input mustn't be null, and just let it throw a NullPointerException if it happens (or you can force one early by starting your method with Objects.requireNonNull(input).


Conclusion: my recommendation is:

List<String> doSomething(String input){
  Objects.requireNonNull(input); // or omit this and let an 
                                 // exception happen further down

  return doMoreProcessingOn(getListfromSomewhereElse(input));
}

It's best if doMoreProcessingOn() produces a new List, rather than modifying input.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.