10
\$\begingroup\$

One of the main problems I have with JList is that no component is actually added to it. This means that things like ActionListeners don't work, and you can't really have a list of interactive components. To solve this, I've tried to quickly implement an alternative.

I have a number of concerns.

  1. Speed: Will this be fast enough to display hundreds (thousands? millions?) of items?

  2. Reliability: I've tried to make it reasonably solid, but no doubt there are a lot of things I should be checking that I am not.

  3. Design: This is probably the most important. I tried to make a general, all-purpose design similar to that of JList, but it feels almost too complex to me. However, I cannot think of a better way. Any suggestions on how to improve it are most welcome.

It has three general parts:

The AdvancedList:

import java.awt.Component;
import java.awt.Container;
import java.awt.Graphics;
import java.util.List;
import java.util.Vector;

import javax.swing.BoxLayout;

public class AdvancedList<E> extends Container {
    protected List<E> model;
    protected List<AdvancedListCell<E>> listCells;
    protected AdvancedCellRenderer<E> cellRenderer;

    public AdvancedList(List<E> model, AdvancedCellRenderer<E> cellRenderer){
        this.setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));

        this.cellRenderer = cellRenderer;
        this.model = new Vector<E>();
        this.listCells = new Vector<AdvancedListCell<E>>();

        for(E element: model){
            this.addElement(element);
        }
    }

    public AdvancedList(AdvancedCellRenderer<E> cellRenderer){
        this(new Vector<E>(), cellRenderer);
    }

    public void addElement(E element){
        AdvancedListCell<E> alc = cellRenderer.getAdvancedListCell(this, element, model.size());
        this.add(alc.getComponent());
        this.listCells.add(alc);
        this.model.add(element);
    }

    public E removeElement(E element){
        int idx = this.model.indexOf(element);
        if(idx != -1) {
            this.remove(idx);
            this.listCells.remove(idx);
            E removed = this.model.remove(idx);
            this.repaint();
            return removed;
        }
        return null;
    }

    public void paint(Graphics g){
        AdvancedListCell<E> tmpListCell = null;
        Component tmpComponent = null;
        for(int i = 0; i < this.model.size(); i++){
            tmpListCell = this.listCells.get(i);
            tmpComponent = this.getComponent(i);
            tmpListCell.updateCell(this, this.model.get(i), i, tmpComponent.hasFocus());
        }
        super.paint(g);
    }
}

The AdvancedCellRenderer:

public interface AdvancedCellRenderer<E> {

    public AdvancedListCell<E> getAdvancedListCell(AdvancedList<? extends E> list, E value, int index);
}

The AdvancedListCell:

import java.awt.Component;

public interface AdvancedListCell<E> {
    public void updateCell(AdvancedList<? extends E> list, E value, 
            int index, boolean cellHasFocus);

    public Component getComponent();
}

And finally, a simple test class:

import java.awt.Component;
import java.awt.Container;
import java.util.Arrays;
import java.util.Vector;

import javax.swing.JFrame;
import javax.swing.JLabel;

public class AdvancedListTest {
    public static void main(String[] args){
        JFrame myFrame = new JFrame();
        Vector<String> myStrings = new Vector<String>();
        myStrings.addAll(Arrays.asList("TEST", "HI", "FOO", "BAR"));
        AdvancedList<String> myList = new AdvancedList<String>(myStrings, new AdvancedCellRenderer<String>(){
            @Override
            public AdvancedListCell<String> getAdvancedListCell(
                    AdvancedList<? extends String> list, String value, int index) {
                return new AdvancedListCell<String>(){
                    private JLabel myLabel = new JLabel();

                    @Override
                    public Component getComponent() {
                        return myLabel;
                    }

                    @Override
                    public void updateCell(AdvancedList<? extends String> list,
                            String value, int index, boolean cellHasFocus) {
                        myLabel.setText(value+", index: "+index+", has focus: "+cellHasFocus);
                    }

                    public String toString(){
                        return "Class: "+myLabel.getClass()+", text: "+myLabel.getText();
                    }

                };
            }

        });
        myFrame.setSize(500, 500);
        myFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        myFrame.add(myList);
        myFrame.revalidate();
        myFrame.repaint();
        myFrame.setVisible(true);
        myList.addElement("Added a string");
        myList.removeElement("HI");
    }
}
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

This is a huge step to take to solve a problem which I am not sure actually exists.

Why do you need to reimplement the way the entire component is rendered just to add the ActionListeners in a different way? I think you have missed something here.

You can override the ListModel (AbstractListModel) and add all the hard effort in to that. As far as I can tell, none of your requirements need to extend beyond that. Your custom ListModel can register actionListeners with each added list member, and can broker the correct action respoinses if needed. Additionally, it can handle all the other events and eventhandling that needs to happen.....

I think all three of your concerns are valid, and you are right to be concerned...

  1. Performance - I am not sure about this one, but I doubt your solution will be as well-refined as the native one.
  2. Reliability - This solution is likely to be buggy, and is not going to be as upgradable in terms of look/feel as other components. This is a big deal.... the native components will be more responsive to that sort of context change.
  3. Over complicated design: I think you are reinventing the wheel here. There is nothing in your design that I can't see in the native implementation.... I think the model/view differentiation is better in the native JList.

All in all, I think you are getting ahead of yourself, and have not fully understood how the native implementation can be adapted to solve the problems you think you have.

\$\endgroup\$
1
\$\begingroup\$

A few notes, mostly about the current code:

  1. Speed: Will this be fast enough to display hundreds (thousands? millions?) of items?

    If I understand correctly I'm afraid that this wouldn't be too user-friendly. If your users need to handle millions of items they might need some kind of filtering, pagination, autocomplete etc. not everything in one huge list.

  2. AFAIK and according to Wikipedia:

    all user interface components should be created and accessed only from the AWT event dispatch thread.

    including myFrame.setSize(500, 500), etc.

  3. Calling overridable methods (like setLayout and addElement) from constructors usually is not a good practice. See What's wrong with overridable method calls in constructors? and Effective Java 2nd Edition, Item 17: Design and document for inheritance, or else prohibit it

  4. I'd use a simple List or ArrayList instead of Vectors. Vector is considered obsolete.

  5. this. is not always necessary (modern IDEs can highlight fields).

  6. public E removeElement(E element) {
        int idx = this.model.indexOf(element);
        if (idx != -1) {
            this.remove(idx);
            this.listCells.remove(idx);
            E removed = this.model.remove(idx);
            this.repaint();
            return removed;
        }
        return null;
    }
    

    I'd use a guard clause here:

    public E removeElement(E element) {
        int idx = model.indexOf(element);
        if (idx == -1) {
            return null;
        }
        remove(idx);
        listCells.remove(idx);
        E removed = model.remove(idx);
        repaint();
        return removed;
    }
    
  7.     public void paint(Graphics g) {
            AdvancedListCell<E> tmpListCell = null;
            Component tmpComponent = null;
            for (int i = 0; i < this.model.size(); i++) {
                tmpListCell = this.listCells.get(i);
                tmpComponent = this.getComponent(i);
                tmpListCell.updateCell(this, this.model.get(i), i, tmpComponent.hasFocus());
            }
            super.paint(g);
        }
    
    • tmpListCell and component could be declared inside the loop.
    • The tmp prefix is unnecessary, I'd call them listCell and component.

      public void paint(Graphics g) { for (int i = 0; i < this.model.size(); i++) { AdvancedListCell listCell = listCells.get(i); Component component = getComponent(i); listCell.updateCell(this, model.get(i), i, component.hasFocus()); } super.paint(g); }

  8. this.model = new Vector<E>();
    this.listCells = new Vector<AdvancedListCell<E>>();
    

    could be assigned in the field declaration:

    protected List<E> model = new Vector<E>();
    protected List<AdvancedListCell<E>> listCells = new Vector<AdvancedListCell<E>>();
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.