0
votes

I have some method which looks awful(especially number of parameters). I wonder how can I make this code cleaner.

The method works with JLists and setting a new model(DefaultListModel). So it just swaps items between two JLists and deletes swapped item in list where was the item taken.

Сriticism and advice are welcome.

Call the method example:

moveToOtherJList(newOrdersModel, newOrdersJList, inProcessOrdersModel, inProcessOrdersJList);

The method:

private void moveToOtherJList(DefaultListModel firstModel, JList firstJList, DefaultListModel secondModel,  JList secondJList)
{
    int selectedIndex = firstJList.getSelectedIndex();
    secondModel.addElement(firstJList.getSelectedValue());
    secondJList.setModel(secondModel);
    firstModel.remove(selectedIndex);
}
1
Since you are not setting firstModel on firstJList, can we assume that it is supposed to be already firstJList’s model? Then, why do you need two parameters? And why is setting secondModel as secondJList’s model part of the method for moving an item? That looks like two entirely unconnected tasks. - Holger

1 Answers

4
votes

I have some method which looks awful(especially number of parameters).

Well there is no need to pass either ListModel, since you can get the ListModel from the JList.

So I would define the method as:

public void moveToOtherJList(JList fromJList, JList toJList)
{
    int selectedIndex = fromJList.getSelectedIndex();
    DefaultListModel fromModel = (DefaultListModel)fromJList.getModel();
    DefaultListModel toModel = (DefaultListModel)toJList.getModel();

    toModel.addElement(fromJList.getSelectedValue());
    fromModel.remove(selectedIndex);
}