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.");
        }
    }
}

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.");
        }
    }
}

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

Code Smell 233 - Collections Count

Maxi Contieri ・ Dec 1 '23

#webdev #javascript #programming #beginners

Disclaimer

Code Smells are my opinion.

Credits

Photo by Valentin Lacoste on Unsplash

---

Good programming is good writing.

John Shore

Software Engineering Great Quotes

Maxi Contieri ・ Dec 28 '20

#codenewbie #programming #quotes #software

---

This article is part of the CodeSmell Series.

How to Find the Stinky parts of your Code

Maxi Contieri ・ May 21 '21

#codenewbie #tutorial #codequality #beginners

Comments

[Vincent A. Cicirello]

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.

[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.

[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.

[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
    }

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.

[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