Undo Fails After Filter: IndexOutOfBoundsException Bug

Introduction

Hey guys! Today, we're diving deep into a rather pesky bug that can pop up when you're using filters and the undo command in your application. Specifically, we're looking at an IndexOutOfBoundsException that occurs when you try to undo an action after applying a filter. This can be super frustrating, especially when you're trying to correct a mistake quickly. So, let's break down the issue, understand why it happens, and explore potential solutions. By the end of this article, you'll have a solid grasp of the problem and how to tackle it head-on.

Problem Description

So, here's the scenario: Imagine you're working with a list of data—let's say, a list of contacts in an address book. You add a few contacts, then you apply a filter to narrow down the list to a specific subset (maybe all the contacts in the "family" group). Now, you decide to undo the last action, perhaps because you added someone with incorrect information. That's when things go south. Instead of smoothly reverting the last action, the application throws an IndexOutOfBoundsException. This exception basically means the application is trying to access an index in a list that doesn't exist, which, in turn, crashes the operation. It's like trying to grab something from an empty shelf—not fun, right?

Steps to Reproduce

To really nail down the problem, let's walk through the exact steps to reproduce this bug. Fire up your application and follow along:

1. Add Initial Contacts: Start by adding a few contacts to your address book. Use the following commands:

   add n/A p/1111 e/a@a.com a/somewhere t/Student
   add n/B p/1111 e/a@a.com a/somewhere t/Student
   add n/C p/1111 e/a@a.com a/somewhere t/Student
   

   These commands add three contacts with names A, B, and C, all with the same phone number, email, address, and tag.

2. Apply a Filter: Next, activate a filter on the left sidebar. In this case, apply the "family" filter. This filter should narrow down your contact list to only the people tagged as "family". If none of the initial contacts are tagged as "family", you might need to modify one of them to include that tag for the filter to show at least one person.

3. Execute the Undo Command: Now, here comes the critical part. Run the undo command:

   undo
   

4. Observe the Error: If everything aligns, you should see the dreaded IndexOutOfBoundsException pop up. The application will likely grind to a halt, displaying an error message similar to the one provided.

Error Message

Here’s the error message you’ll likely encounter:

Exception in thread "JavaFX Application Thread" java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
    at javafx.fxml/javafx.fxml.FXMLLoader$MethodHandler.invoke(FXMLLoader.java:1863)
    at javafx.fxml/javafx.fxml.FXMLLoader$ControllerMethodEventHandler.handle(FXMLLoader.java:1731)
    ...
Caused by: java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    ...
Caused by: java.lang.IndexOutOfBoundsException
    at javafx.base/javafx.collections.transformation.FilteredList.get(FilteredList.java:169)
    at seedu.address.logic.commands.EditCommand.undo(EditCommand.java:149)
    at seedu.address.logic.commands.UndoCommand.execute(UndoCommand.java:50)
    at seedu.address.logic.LogicManager.execute(LogicManager.java:53)
    at seedu.address.ui.MainWindow.executeCommand(MainWindow.java:176)
    at seedu.address.ui.CommandBox.handleCommandEntered(CommandBox.java:68)
    ...

Root Cause Analysis

The million-dollar question is, why does this happen? The key lies in how the FilteredList (the component responsible for applying filters) interacts with the undo command. When you apply a filter, the FilteredList creates a transformed view of the original list. This view only includes the elements that match the filter criteria. The problem arises when the undo command tries to revert an action based on indices that are valid in the filtered view but no longer valid in the underlying, unfiltered list. This discrepancy leads to the IndexOutOfBoundsException.

Technical Deep Dive

Let's break down the technical aspects of this issue. The stack trace points to FilteredList.get(FilteredList.java:169) as the origin of the IndexOutOfBoundsException. This indicates that the get() method of the FilteredList is being called with an invalid index. The undo() method in EditCommand is trying to revert changes, and it's using indices that are correct within the filtered list but not in the master list. This happens because the UndoCommand likely doesn't account for the filtering that has been applied.

Understanding FilteredList

The FilteredList is a dynamic view of a list that applies a predicate to determine which elements are visible. When you add, delete, or modify elements in the underlying list, the FilteredList automatically updates its view. However, the indices in the FilteredList are relative to the filtered view, not the underlying list. This is where the problem starts when using undo.

Examining the UndoCommand

The UndoCommand is responsible for reverting the last command executed. In this case, it needs to revert an EditCommand. The EditCommand likely stores the index of the element being edited. When undo is called, it uses this index to revert the changes. However, if a filter is applied, the index stored in the EditCommand may no longer be valid in the current filtered view, leading to the IndexOutOfBoundsException.

Potential Solutions

Alright, enough about the problem – let's talk solutions! Here are a few approaches to fix this IndexOutOfBoundsException:

Option 1: Convert Indices to Account for Filtering

One way to solve this is to convert the indices used in the UndoCommand to account for the filtering. Before executing the undo, you can map the filtered index back to the original index in the unfiltered list. This ensures that the UndoCommand operates on the correct element. Here’s a conceptual example:

1. Store Original Index: When an EditCommand is executed, store both the filtered index and the original index.
2. Map Index on Undo: In the UndoCommand, use the filtered index to find the corresponding original index before reverting the changes.

This approach ensures that the UndoCommand always operates on the correct element, regardless of any active filters.

Option 2: Operate on the Unfiltered List Directly

Another approach is to have the UndoCommand operate directly on the unfiltered list. This means bypassing the FilteredList altogether when reverting changes. This can be achieved by:

1. Access the Master List: Ensure the UndoCommand has access to the master, unfiltered list.
2. Revert Changes Directly: Use the original index to revert changes directly in the master list.
3. Refresh the Filtered View: After reverting the changes, refresh the FilteredList to reflect the updated state.

This approach simplifies the logic by avoiding the need to map indices, but it requires careful management of the master list and the filtered view.

Option 3: Disable Undo When Filters Are Active

A simpler, albeit less user-friendly, solution is to disable the undo command when filters are active. This prevents the IndexOutOfBoundsException from occurring, but it also limits the user's ability to revert changes when filters are in use. Here’s how you can implement this:

1. Check Filter State: Before executing the UndoCommand, check if any filters are active.
2. Disable Undo: If filters are active, disable the UndoCommand and display a message to the user explaining why it’s disabled.

This approach is the easiest to implement, but it may not be the best user experience.

Code Examples

To illustrate these solutions, let's look at some simplified code examples.

Example of Option 1: Converting Indices

public class EditCommand {
    private int filteredIndex;
    private int originalIndex;

    public EditCommand(int filteredIndex, int originalIndex) {
        this.filteredIndex = filteredIndex;
        this.originalIndex = originalIndex;
    }

    public void undo() {
        // Use originalIndex to revert changes in the master list
    }
}

public class UndoCommand {
    public void execute() {
        EditCommand lastEdit = getLastEditCommand();
        lastEdit.undo();
    }
}

Example of Option 2: Operating on the Unfiltered List

public class UndoCommand {
    private List<Person> masterList;

    public UndoCommand(List<Person> masterList) {
        this.masterList = masterList;
    }

    public void execute(int originalIndex) {
        // Revert changes directly in the masterList using originalIndex
        // Refresh the FilteredList
    }
}

Testing and Validation

After implementing any of these solutions, it’s crucial to test and validate that the IndexOutOfBoundsException is resolved and that the undo command works correctly under various filtering scenarios. Here are some test cases to consider:

1. Undo After Applying a Filter: Apply a filter and then execute the undo command. Verify that the changes are reverted correctly.
2. Undo After Removing a Filter: Apply a filter, execute the undo command, and then remove the filter. Verify that the changes are still reverted correctly.
3. Undo Multiple Commands: Execute multiple commands, apply a filter, and then execute the undo command multiple times. Verify that all changes are reverted correctly.

Conclusion

So, there you have it, guys! We’ve taken a deep dive into the IndexOutOfBoundsException that can occur when using the undo command with filters. We've explored why this happens and presented several solutions to tackle the issue. Whether you choose to convert indices, operate directly on the unfiltered list, or disable the undo command when filters are active, the key is to understand the interaction between the FilteredList and the UndoCommand. With a bit of careful coding and thorough testing, you can ensure a smooth and error-free user experience. Happy coding, and may your undo commands always work as expected!