DEV Community

Cover image for Code Smell 251 - Collections Empty
Maxi Contieri
Maxi Contieri

Posted on • Originally published at maximilianocontieri.com

Code Smell 251 - Collections Empty

Do you want to count collections or know if they are empty?

TL;DR: Use declarative names. Always

Problems

  • Readability

  • Cache Invalidation

  • Performance Penalties

  • Type Safety

Solutions

  1. Replace count() == 0 and size()==0 usages

Context

isEmpty() and count()==0 seem to be equivalent but have deep differences.

The semantics are clearer.

Skipping this declarative method violating the collection encapsulation might have performance issues.

Sample Code

Wrong

import java.util.EmptyStackException;
import java.util.Stack;

public class SchrodingerStack<T> {
    private Stack<T> stack;

    public SchrodingerStack() {
        stack = new Stack<>();
    }

    public void push(T item) {
        stack.push(item);
    }

    public T pop() {
        if (stack.size() == 0) {
            throw new EmptyStackException();
        }

        T item = stack.pop();
        return item;
    }

    public int size() {
        return stack.size();
        // This has O(n) linear time
        // And the stack might not be fully reachable in memory
        // While you wait, the stack isEmpty and notEmpty 
        // at the same time
    }

    public static void main(String[] args) {
        SchrodingerStack<String> stack = new SchrodingerStack<>();

        stack.push("Siamese");
        stack.push("Garfield"); 

        while (stack.size() > 0) {
            System.out.println("Popped element: " + stack.pop());
        }

        if (stack.size() == 0 ) {
            // Less readable
            // violating encapsulation
            // and coupled to the implementation
            System.out.println("The stack is empty.");
        } else {
            System.out.println("The stack is not empty.");
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

Right

import java.util.EmptyStackException;
import java.util.Stack;

public class SchrodingerStack<T> {
    private Stack<T> stack;
    private boolean isEmpty;

    public SchrodingerStack() {
        stack = new Stack<>();
        isEmpty = true;
    }

    public void push(T item) {
        stack.push(item);
        isEmpty = false; 
    }

    public T pop() {
        if (isEmpty()) {
            throw new EmptyStackException();
        }

        T item = stack.pop();
        if (stack.isEmpty()) {
            isEmpty = true;             
        }
        return item;
    }

    public boolean isEmpty() {
        return isEmpty;
        // This has O(1) constant time
    }

    public int size() {
        return stack.size();
        // This has O(n) linear time
        // And the stack might not be fully reachable in memory
        // While you wait, the stack isEmpty and notEmpty 
        // at the same time
    }

    public static void main(String[] args) {
        SchrodingerStack<String> stack = new SchrodingerStack<>();

        stack.push("Siamese");
        stack.push("Garfield"); 

        while (!stack.isEmpty()) {
            System.out.println("Popped element: " + stack.pop());
        }

        if (stack.isEmpty()) {
            // Semantic operation not violating encapsulation
            System.out.println("The stack is empty.");
        } else {
            System.out.println("The stack is not empty.");
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

Detection

[X] Automatic

You can check for this expression using syntax abstraction trees.

Tags

  • Readability

Level

[X] Beginner

AI Generation

LLMs generate abstractions using empty() functions

AI Detection

Gemini detected the problem of using count() == 0

Conclusion

Using IsEmpty() is recommended for checking if a collection is empty due to its clarity and potential performance benefits.

Relations

Disclaimer

Code Smells are my opinion.

Credits

Photo by Valentin Lacoste on Unsplash


Good programming is good writing.

John Shore


This article is part of the CodeSmell Series.

Top comments (5)

Collapse
 
cicirello profile image
Vincent A. Cicirello • Edited

I definitely agree with your rationale of clarity for using isEmpty(). However, there is no performance difference. Java's Stack class extends Vector, which maintains number of elements in an instance field, and the size() method is a simple getter with an O(1) runtime. Your O(n) comment in examples is incorrect. There isn't any performance difference because the same operations are done (the isEmpty() method checks the same field that the size() method returns). The benefit of isEmpty() is purely readability, which of course is sufficient motivation.

Your example of the "right" way is actually duplicating functionality that is already in Java's Stack class. Your isEmpty() should just call stack.isEmpty() and you can eliminate the isEmpty field and all other logic associated with it.

Collapse
 
ervin_szilagyi profile image
Ervin Szilagyi

Yeah, this whole example is questionable.

Might be beside the point, but I don't know why would anybody use Stack or Vector in modern Java code. The recommended way for a stack in Java would be to use Deque with an implementation like ArrayDeque or LinkedList. You might prefer the first one. Both of these implement the isEmpy method inherited from Collection interface. Both implementation run in O(1) time.

Nevertheless, I agree with a premise of this post. I think it is a good practice to have a method for checking the emptiness of a collection.

Collapse
 
cicirello profile image
Vincent A. Cicirello

One reason someone might prefer Stack over one of the Deque implementations is in a context where you need a LIFO data structure. Someone reading code that is using a Stack immediately knows it is LIFO. With a Deque since removals and additions can be at both ends, you need more context to determine how it is being used. Is it being used like a stack, or a queue, or as its name implies like a double-ended queue? Stack expresses intent more clearly than Deque in a LIFO context.

Collapse
 
moopet profile image
Ben Sinclair
    public T pop() {
        if (isEmpty()) {
            throw new EmptyStackException();
        }

        T item = stack.pop();
        if (stack.isEmpty()) {
            isEmpty = true;             
        }
        return item;
    }

    public boolean isEmpty() {
        return isEmpty;
        // This has O(1) constant time
    }
Enter fullscreen mode Exit fullscreen mode

If you pop an item, it'll call the isEmpty() method, which returns the class variable isEmpty. If that's true, then it'll set the class variable to true. Isn't that tautological?

It feels like the isEmpty() method should wrap the size or count you're trying to avoid, else this won't work.

Collapse
 
mcsee profile image
Maxi Contieri

Thank you very much

The main challenge is about readabality and not performance.
I'm not an expert in Java and I didn't know this facts so they are good points