6
\$\begingroup\$

I have a class called Piece, and many many subclasses of Piece. I want to add an instance of every single subclass of Piece (under the pieces package) to my JTree. Currently, I have this class with a huge function (well, 57 lines, but still) that increases in size every time I add functionality to my program.

My initial approach to this was "Hey, let's just use reflection or something to find out all of the classes under the package pieces and add them to the tree!" but this SO question shot that down. My second, working approach is to add them all manually by hand. This seems like too much work though and it seems like there would be a better way to do this.

As always, miscellaneous comments on my code are very welcome.

initTree()

private void initTree(final UserInterface userInterface) {
    tree = new JTree(createTree());
    tree.setRootVisible(false);
    tree.getSelectionModel().setSelectionMode(TreeSelectionModel.SINGLE_TREE_SELECTION);

    tree.addTreeSelectionListener(new TreeSelectionListener(){
        @Override
        public void valueChanged(TreeSelectionEvent e) {
             DefaultMutableTreeNode node = (DefaultMutableTreeNode)
                       tree.getLastSelectedPathComponent();

             if (node == null){
                 return;
             }

             if (node.isLeaf() && node.getUserObject() instanceof Piece) {
                 Piece pieceCreated = (Piece) ((Piece)node.getUserObject()).getInstance();
                 userInterface.space.addPiece(pieceCreated);
             }
        }
    });
    add( new JScrollPane(tree), BorderLayout.CENTER );
}

createTree()

private DefaultMutableTreeNode createTree(){
    //create the root node
    DefaultMutableTreeNode root = new DefaultMutableTreeNode("Root");
    //create the child nodes
    DefaultMutableTreeNode gatesNode = new DefaultMutableTreeNode("Gates");

        DefaultMutableTreeNode arithmeticNode = new DefaultMutableTreeNode("Arithmetic");
        arithmeticNode.add(new DefaultMutableTreeNode(new Add(0,0)));
        arithmeticNode.add(new DefaultMutableTreeNode(new Subtract(0,0)));
        arithmeticNode.add(new DefaultMutableTreeNode(new Multiply(0,0)));
        arithmeticNode.add(new DefaultMutableTreeNode(new Divide(0,0)));
        arithmeticNode.add(new DefaultMutableTreeNode(new Modulo(0,0)));
        arithmeticNode.add(new DefaultMutableTreeNode(new Random(0,0)));
        gatesNode.add(arithmeticNode);

        DefaultMutableTreeNode bitwiseNode = new DefaultMutableTreeNode("Bitwise");
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseAnd(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseNand(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseNor(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseNot(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseOr(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseXor(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseXnor(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseLeftshift(0,0)));
        bitwiseNode.add(new DefaultMutableTreeNode(new BitwiseRightshift(0,0)));
        gatesNode.add(bitwiseNode);

        //etc with more subclasses of Piece
    return root;
}

Piece.java

public abstract class Piece implements Serializable{
    private static final long serialVersionUID = 2022414861350098747L;
    private static final Color BACKGROUND_COLOR = new Color(200,200,200);
    static final BasicStroke PORT_STROKE = new BasicStroke(1);

    protected int x, y, width, height;
    protected Input input;
    protected Output output;

    public Piece(int x, int y, int width, int height, int inputs, int outputs){
       this.x = x;
       this.y = y;
       this.width = width;
       this.height = height;
       this.input = new Input(inputs, this);
       this.output = new Output(outputs, this);
    }
    public abstract void draw(Graphics2D g);
    public void drawBackground(Graphics2D g){
        g.setColor(BACKGROUND_COLOR);
        g.fillRoundRect(x, y, width, height,10,10);
        input.draw(g);
        output.draw(g);
    }
    public void drawConnections(Graphics2D g){
        output.drawConnections(g);
    }
    public void connect(Piece other, int outputPort, int inputPort){
        output.connect(other, outputPort, inputPort);
        update();
    }
    public void disconnect(final int outputPort){
        output.disconnect(outputPort);
        update();
    }
    public void update(){
        output.update();
    }
    public abstract Value send(int outputPort);
    public abstract void recieve(int inputPort, Value v);
    public abstract void doubleClicked();
    public abstract Piece getInstance();
    public String toString(){
        return this.getClass().getSimpleName();
    }
    public void setPosition(final Point point) {
        x = point.x;
        y = point.y;
    }
    public boolean contains(final Point p){
        return  x < p.getX() && 
                y < p.getY() &&
                x + width > p.getX()  &&
                y + height > p.getY();
    }
    public Integer getOutputPortFromPoint(final Point p){
        if(output.contains(p)){
            if(output.getConnections().length == 0)
                return null;

            return output.getOuputFromY(p.y);
        }
        return null;
    }
    public Integer getInputPortFromPoint(final Point p){
        if(input.contains(p)){
            if(input.getConnections().length == 0){
                return null;
            }

            return input.getInputFromY(p.y);
        }
        return null;
    }
    public int getX() {
        return x;
    }
    public int getY() {
        return y;
    }
    public int getWidth(){
        return width;
    }
    public int getHeight(){
        return height;
    }
    public Point getPointFromOutputPort(int portSelected) {
        return output.getPointFromPort(portSelected);
    }
}

Add.java (Example subclass of Piece)

public class Add extends Piece{
    private static final long serialVersionUID = -1786155220275379870L;
    //v1 + v2 = v
    private ValueInteger param1;
    private ValueInteger param2;
    private ValueInteger result;
    private Title title;
    public Add(int x, int y) {
        super(x,y,150,75, 2, 1);
        title = new Title("Add", this);
        param1 = new ValueInteger(0);
        result = new ValueInteger(0);
        param2 = new ValueInteger(0);
    }

    @Override
    public void draw(Graphics2D g) {
        drawBackground(g);
        title.draw(g);
        g.setColor(Color.BLUE);
        if(result != null)
            g.drawString(result.toString(), x + 10, y + 20);
    }

    @Override
    public Value send(int outputPort) {
        return result;
    }

    @Override
    public void recieve(int inputPort, Value v) {
        if(inputPort == 0)
            param1 = (ValueInteger) v;
        else if (inputPort == 1)
            param2 = (ValueInteger) v;
        this.result = new ValueInteger(param1.add(param2));
    }

    @Override
    public void doubleClicked() {
        // TODO Auto-generated method stub
    }
    @Override
    public Piece getInstance() {
        return new Add(10,10);
    }
}
\$\endgroup\$
  • \$\begingroup\$ What exactly is a Piece? Perhaps you mean LogicComponent? The fact that you have so many subclasses is a sign that maybe inheritance is not the best approach. There might be a violation of the Single Responsibility Principle. Or, maybe your application needs to be more data-driven than code-driven. \$\endgroup\$ – 200_success Apr 30 '14 at 20:20
5
\$\begingroup\$
public enum PieceGroups{

    BITWISE(BitwiseAnd.class, BitwiseNand.class, BitwiseNor.class, BitwiseNot.class,
            BitwiseOr.class, BitwiseXor.class, BitwiseXnor.class, BitwiseLeftshift.class,
            BitwiseRightshift.class),
    ARITHMETIC(Add.class, Substract.class, Multiply.class, Divide.class, Modulo.class, Random.class);

    private Set<Piece> classSet = new HashSet<Piece>();

    private PieceGroups(Class... classes) {
        for (Class theClass : classes) {
            try {
                Constructor construtor = theClass.getDeclaredConstructor(int.class, int.class);
                classSet.add((Piece) construtor.newInstance(0, 0));
            } catch (NoSuchMethodException ex) {
                processException();
            } catch (SecurityException ex) {
                processException();
            } catch (InstantiationException ex) {
                processException();
            } catch (IllegalAccessException ex) {
                processException();
            } catch (IllegalArgumentException ex) {
                processException();
            } catch (InvocationTargetException ex) {
                 processException();
            }
        }
    }

    public Set<Piece> getClasses () {
        return classSet;
    }

    private void processException () {
        throw new IllegalArgumentException("Something went wrong with the init of the Enum");
    }
}

Explication :

We create an enum, still the best singleton what there is.
Then the states of the enum are grouping where the Piece (sub)class can have, like the Bitwise and Arithmetic you showed.

Constructor of the enum is vararg, so you can give up as many classes as you want with one group.

We create an instance of the class in the constructor with reflection. At the moment I saw that all your classes have an constructor with 2 ints, so I rely that is for all the implementations of like that.

An IllegalArgumentException is thrown when your enum can't be created by an fault (class not found, constructor not present,...)
You can also just skip that class at instanciation but then you never know when something went wrong.

At last, just ask PieceGroups.BITWISE.getClasses() and you will have a set of the corresponding group.

\$\endgroup\$
  • \$\begingroup\$ Thank you, while I still have to hardcode all of the classes in, it's a very clean solution. \$\endgroup\$ – Kyranstar Apr 30 '14 at 21:15
  • 1
    \$\begingroup\$ In Java 7, you can handle multiple kinds of exceptions in one catch block. \$\endgroup\$ – 200_success Apr 30 '14 at 21:28
  • \$\begingroup\$ @200_success you are very correct. When I was creating that class I was thinking of that, but IDE is configured java 6, so for not making faults I go with java 6. \$\endgroup\$ – chillworld May 1 '14 at 5:42
  • \$\begingroup\$ Instead of supplying the class and then using reflection, consider adding an instance directly: BITWISE(new BitwiseAnd(0,0), new BitwiseNand(0,0), ...). The class isn't used but for instantiating through reflection anyway, and that saves us having to guess the constructors. \$\endgroup\$ – JvR May 6 '14 at 11:30
3
\$\begingroup\$

Depending on your needs, there are ways around this but, sadly, none of them will work automagically, and they may require some effort. Only you can decide what's warranted and what's overkill.

From what I can see, you're building a GUI for chips or circuits. I imagine you want to provide a component that lists gates for your users, much like the component buttons in WindowBuilder or Matisse. Let's see how these solve the problem.

Asking for directions

WindowBuilder comes with the standard Swing and AWT components out of the box. Since these will be in pretty much any Java distribution, they can just add the components manually once. It's a bit tedious to do, but you only need to do it once (and maintain forever).

What about components that aren't standard, like when your users make stuff of their own? Well, there are three main ways that jump out:

  1. Ask your platform for an index. For WindowBuilder, this is Eclipse, which has information on loaded and available classes. For stand-alone Java applications like GUIs, this will involve scanning your class path.

    Also available for your needs is java.util.ServiceLoader. If you or your users can be bothered to make an index file in provided JARs, ServiceLoader will do some lifting for you.

  2. Ask your user for a JAR and scan it. Open it up, check each class whether it extends Piece, and add/flag it if it does. This covers archives that aren't on the class path.

  3. Ask your user for a class name and let the ClassLoader fetch it. This covers classes that are not in a JAR or on the class path.

Once you have potentially valid types, it's time to figure out how to instantiate and present them.

Reflecting on what you found

So suppose you have a list of classes that extend Piece and we may or may not be able to accommodate. Unfortunately, Piece.getInstance() won't help us here: to be overridable, it needs to be an instance method; to use an instance method, we need an instance. Egg, meet Chicken.

Much like we'd do with JavaBeans, we can gleam some information from your classes using reflection and convention:

  1. Find a public static method matching a specific signature, e.g. a factory method. create(), createInstance(), and so on, are good candidates. Document what you accept, and in what order you search.

  2. Find all public constructors, and try to invoke them with 'default' values. What constitutes default values depends a bit on context. We may need to ask our users to provide values for us. Again, document what is supported, and how you will try instantiating.

After we've done all this, we pretty much have what we need to allow our users to build whatever they want. But GUI builders usually don't forget coordinates to custom components after reloading, so how do we save our users the trouble not to have to input them over and over again?

Maintain a registry (optional)

We should save two things about found implementations: (1) their class name, so we can find them, and (2) how to instantiate them. How you want to save this is up to preference; I'd lean towards an XML file because that allows enterprising users to tweak, change, or add others if we somehow fail to find them.

<?xml version="1.0" encoding="utf-8"?>
<pieces version="1">
  <piece>
    <className>com.acme.pieces.Add</className>
    <factory-method name="create">
    </factory-method>
  </piece>
  <piece>
    <className>com.acme.pieces.Explode</className>
    <constructor>
      <arg type="int">10</arg>
      <arg type="int">10</arg>
    </constructor>
  </piece>
</pieces>

That's really all we need to store. (Do version your data format, even if you plan on only ever supporting one.)

Worth it?

If we're going to be user-friendly and fail-safe and all these beautiful things, this is a lot of effort, and possibly even overkill. This feels like a basic enough issue that there must be some projects with some support for it (I'm looking sideways at Spring and Apache, here).

If you're just going for a fixed list of things that you want available, manually maintaining a list in code is pretty much your simplest, if sometimes tedious, solution. It's not shiny or pretty but it's quick and it may meet your needs.

\$\endgroup\$
2
\$\begingroup\$

You will need to maintain a list of what things are in your pieces package manually.

Best way is to do what you do, that is add them one by one to some sort of container.

However selecting them is a separate task from adding them to your tree

static List<Piece> piecesIWant() {
    List<Piece> pieces = new ArrayList<Piece>();
    pieces.add(new Add(0,0));
    pieces.add(new Subtract(0,0));
    return pieces;
}

void createTree(Collection<Piece> wantedPieces) {
    DefaultMutableTreeNode arithmeticNode = new DefaultMutableTreeNode("Arithmetic");
    for (Piece p : wantedPieces) {
        arithmeticNode.add(new DefaultMutableTreeNode(p));
    }
}
\$\endgroup\$
2
\$\begingroup\$

My solution to keeping track of Piece instances would be a enum PieceRepository.

import java.util.ArrayList;
import java.util.List;


public enum PieceRepository {

    INSATNCE;

    private List<Piece> pieces;

    private PieceRepository() {
        pieces = new ArrayList<Piece>();
    }

    public void addPiece(Piece piece) {
        pieces.add(piece);
    }

    public List<Piece> getPieces() {
        return pieces;
    }

}

Piece sample code:

public class Piece {

    public Piece() {
        PieceRepository.INSTANCE.addPiece(this);
    }

}

Some other class that extends Piece:

public class HalfPiece extends Piece {

    public HalfPiece() {

    }

}

And test:

public class TestMain {

    public static void main(String[] args) {

        new Piece();
        new Piece();
        new Piece();
        new HalfPiece();

        System.out.println(PieceRepository.INSTANCE.getPieces().size());

    }

}

Output is:

4

Edit: Used https://stackoverflow.com/questions/3635396/pattern-for-lazy-thread-safe-singleton-instantiation-in-java To make repository thread safe.

\$\endgroup\$
  • \$\begingroup\$ The repository is not a thread-safe singleton, and Piece() publishes a reference before it leaves the constructor, which may leave it in an inconsistent state for other threads. This may become an issue since the OP is using Swing. \$\endgroup\$ – JvR Apr 30 '14 at 12:21
  • \$\begingroup\$ Updated my answer regards your first note, though at the moment can't think of fix for second. \$\endgroup\$ – Karolis Šarapnickis Apr 30 '14 at 13:23
  • \$\begingroup\$ Preventing the self-publishing would need to route creation through a factory method that first constructs and then registers the instance. So instead of new Piece() it will be Piece.create() or so. It's a slight hassle, but tracking down concurrency bugs is a bit more of a hassle. ;) \$\endgroup\$ – JvR Apr 30 '14 at 15:57
1
\$\begingroup\$

I have attempted something similar to this, that (mostly) got rid of the hardcoded registration problem, which was based on using static initialization blocks and class loaders.

In my case, each class was responsible for generating a binary encoding for some data, in varying formats.

First of all, I had a class that would accept the encoder definition

class Encoders {
    public void addEncoding(String identifier, Constructor<Encoder> ctor){
        // ...
    }
}

Each encoder class looked something like this:

class MyEncoder {
    static {
        Encoders.addEncoding("MyEncoding", MyEncoder.class.getDeclaredConstructor());
    }

    public MyEncoder(){
        // ...
    }
}

Then to load them all, I iterated through the classes specified in an external config file containing a list of the binary class names for each encoder. One could do something even fancier by processing these configs in different files.

public void loadEncoders(Logger log){
    try(BufferedReader reader = new BufferedReader(new FileReader("encoders.txt"))){
        loadClasses(reader, log);
    } catch (FileNotFoundException e) {
        log.log(Level.WARNING, "Could not find encoders list!", e);
    } catch (IOException e){
        log.log(Level.WARNING, "Could not read encoders list!", e);
    }
}

public boolean loadClasses(BufferedReader reader, Logger log){
    ClassLoader loader = ClassLoader.getSystemClassLoader();
    boolean success = true;
    while(true){
        String name = reader.readLine();
        if(name == null) break;

        try {
            loader.loadClass(name);
        } catch (ClassNotFoundException e){
            log.log(Level.WARNING, "Unable to find " + name, e);
            success = false;
        }
        return success;
    }
}
\$\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.