HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Delete item in List A will also delete corresponding items in List B

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
itemdeletealsowillitemslistcorresponding

Problem

This Java Swing code consists of 2 JLists and a delete button. When I select an item from locationList, it will also delete items from branchList. This is achieved by using the equals method to compare object between the two lists.

Is there any better solution/data structure that I can implement where when I delete an object in List A, it will also delete object in List B which is linked to it?

Branch class:

public class Branch {
private String loc;
private String branch;

public Branch() {

}

public Branch(String loc, String branch) {
    this.loc = loc;
    this.branch = branch;

}

public String toString() {
    return "(" + loc + ") - " + branch;
}
public String getLoc() {
    return loc;
}


Location class:

public class Location {
private Long locCount;
private String loc;

public Location(String loc, Long locCount) {
    this.locCount = locCount;
    this.loc = loc;
}

public String toString() {
    return loc + " (" + locCount + ")";
}

public String getLocation() {
    return loc;
}


Main class:

```
import java.awt.EventQueue;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import javax.swing.DefaultListModel;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JList;
import javax.swing.JOptionPane;
import javax.swing.JScrollPane;

public class Test {

private JFrame frame;
private List locationCount;
private Long total;
private Map showLocation;

/**
* Launch the application.
*/
public static void main(String[] args) {
EventQueue.invokeLater(new Runnable() {
public void run() {
try {
Test window = new Test();
window.frame.setVisible(true);
} catch (Exception e) {
e.printStackTrace();
}
}
});
}

/**
* Create the application.
*/
public Test() {

Solution

In my comment I mentioned that this frame looks like a demo of what you are trying to achieve. After diving into the code though I feel compelled to mention a few things that will make what you are trying to do hopefully easier. First thing is to use methods to help make clear what you are trying to do from a higher level. Here is a small idea that I've been playing with

private void initialize() {
    JPanel listsPanel = new JPanel(new BorderLayout());

    DefaultListModel branches = initializeBranchesPanel(listsPanel);
    DefaultListModel location = initializeLocationPanel(listsPanel);
    JButton btnDelete = initialzeDeleteButtonPanel();

    add(listsPanel, BorderLayout.NORTH);
    add(btnDelete, BorderLayout.SOUTH);

    putDataIntoBranches(branches);
    //...
}


Lukasz Zwierko mentioned a design pattern in his comment to you (MVVM to be specific) This is a very helpful aspect to making the code easier to write, and understand. It separates concern of certain bits of code into different classes that describe their intention. This will make it easier to come back to later and adjust the appropriate class for changed functionality/bug fixes. I would highly suggest looking into MVP (but other popular patterns are the MVVM, and the older MVC). It would take too long for me to give an example of how to do this.

Next I would also like to mention unit tests. Your problem at hand uses too much code to give you the desired results. I know you can feel it because you asked your question here. Unit tests are great because they are small bits of code that can help mold how your code works. There is a art to it, and it needs practice to get good at it. That said there are TONS of resources out there and how to write unit tests. The following two tests are what I wrote to show that I only use one source of data and update the other two.

public class BranchDataTest {
    private BranchData data;
    @BeforeMethod
    public void setup(){
        data = new BranchData();
    }

    @Test
    public void testWhenNewBranchIsAddedNewLocationIsAdded(){
        Branch branch = new Branch("Location", "Branch");
        data.addBranch(branch);

        assertEquals(data.getLocations().size(), 1);
        Location location = data.getLocations().get(0);

        assertEquals(location.toString(), "Location (1)");
    }

    @Test
    public void testWhenMultipleBranchesExistLocationCountIsIncreased(){
        data.addBranch(new Branch("Location", "Branch1"));
        data.addBranch(new Branch("Location", "Branch2"));

        assertEquals(data.getLocations().size(), 1);
        Location location = data.getLocations().get(0);

        assertEquals(location.toString(), "Location (2)");
    }

}


There are probably more and better ways to write the test, but this just shows where I started. FYI in case you are curious what my new BranchData class looks like (it isn't complete it is just enough to make those 2 tests pass)

public class BranchData implements ListDataListener {
    private DefaultListModel branches = new DefaultListModel<>();
    private DefaultListModel locations = new DefaultListModel<>();

    public BranchData() {
        branches.addListDataListener(this);
    }

    public void addBranch(Branch branch) {
        branches.addElement(branch);
    }

    public DefaultListModel getLocations() {
        return locations;
    }

    @Override
    public void intervalAdded(ListDataEvent e) {
        //debating about just using branches, or using the getSource(),
        //for now I'll keep this modular.
        Branch branchAdded = ((DefaultListModel) e.getSource())
                .elementAt(e.getIndex0());

        if (updatedExistingLocationCount(branchAdded)) return;

        locations.addElement(new Location(branchAdded.getLocation(), 1));
    }

    private boolean updatedExistingLocationCount(Branch branchAdded) {
        for (int i = 0; i < locations.size(); i++) {
            Location location = locations.get(i);
            if(location.getLocation()==branchAdded.getLocation()){
                location.increaseCount();
                return true;
            }
        }
        return false;
    }

    @Override
    public void intervalRemoved(ListDataEvent e) {
    }

    @Override
    public void contentsChanged(ListDataEvent e) {
    }
}


Those two tests pass in 23ms, and once I get that class fully built I can know for certain that whatever I add (and delete) that my list will update as needed. Hope this helps guide you to a better solution.

Code Snippets

private void initialize() {
    JPanel listsPanel = new JPanel(new BorderLayout());

    DefaultListModel<Branch> branches = initializeBranchesPanel(listsPanel);
    DefaultListModel<Location> location = initializeLocationPanel(listsPanel);
    JButton btnDelete = initialzeDeleteButtonPanel();

    add(listsPanel, BorderLayout.NORTH);
    add(btnDelete, BorderLayout.SOUTH);

    putDataIntoBranches(branches);
    //...
}
public class BranchDataTest {
    private BranchData data;
    @BeforeMethod
    public void setup(){
        data = new BranchData();
    }

    @Test
    public void testWhenNewBranchIsAddedNewLocationIsAdded(){
        Branch branch = new Branch("Location", "Branch");
        data.addBranch(branch);

        assertEquals(data.getLocations().size(), 1);
        Location location = data.getLocations().get(0);

        assertEquals(location.toString(), "Location (1)");
    }

    @Test
    public void testWhenMultipleBranchesExistLocationCountIsIncreased(){
        data.addBranch(new Branch("Location", "Branch1"));
        data.addBranch(new Branch("Location", "Branch2"));

        assertEquals(data.getLocations().size(), 1);
        Location location = data.getLocations().get(0);

        assertEquals(location.toString(), "Location (2)");
    }

}
public class BranchData implements ListDataListener {
    private DefaultListModel<Branch> branches = new DefaultListModel<>();
    private DefaultListModel<Location> locations = new DefaultListModel<>();

    public BranchData() {
        branches.addListDataListener(this);
    }

    public void addBranch(Branch branch) {
        branches.addElement(branch);
    }

    public DefaultListModel<Location> getLocations() {
        return locations;
    }

    @Override
    public void intervalAdded(ListDataEvent e) {
        //debating about just using branches, or using the getSource(),
        //for now I'll keep this modular.
        Branch branchAdded = ((DefaultListModel<Branch>) e.getSource())
                .elementAt(e.getIndex0());

        if (updatedExistingLocationCount(branchAdded)) return;

        locations.addElement(new Location(branchAdded.getLocation(), 1));
    }

    private boolean updatedExistingLocationCount(Branch branchAdded) {
        for (int i = 0; i < locations.size(); i++) {
            Location location = locations.get(i);
            if(location.getLocation()==branchAdded.getLocation()){
                location.increaseCount();
                return true;
            }
        }
        return false;
    }

    @Override
    public void intervalRemoved(ListDataEvent e) {
    }

    @Override
    public void contentsChanged(ListDataEvent e) {
    }
}

Context

StackExchange Code Review Q#96817, answer score: 2

Revisions (0)

No revisions yet.