2

A Java project contains method to draw an "Edit" button. Can you please explain the benefit of using an interface here? What an interface can do what a more simple JBotton button = new JButton(); cannot?

private ValueObject<?> getButtonValueObject() {
        ValueObject<?> buttonValue = new MutableValueObject<Object>() {
            private String title = "Edit"; 

            @Override
            public Object getValue() {
                return new AbstractAction(title) {   
                    public void actionPerformed(ActionEvent e) {
                        final JDialog dialog = new JDialog();
                        dialog.setTitle("Values");
                        dialog.setLayout(new BorderLayout());                        
                        JPanel panel = new JPanel();
                        dialog.add(panel, BorderLayout.NORTH);
                        .......

                        dialog.setVisible(true);
                    }

                    @Override
                    public String toString() {
                        return title;
                    }
                };
            }
        };
        return buttonValue;
    }


//ValueObject.java
public interface ValueObject<T> {
    T getValue();
}
sixtytrees
  • 1,156
  • 1
  • 10
  • 25
  • That seems to be out of context and thus is hard to explain. Isn't there a documentation for that project that you can refer to? – Thomas Jul 21 '16 at 14:34
  • @Thomas Not really. Can you give a couple of examples when such interfaces can be useful. It is OK if they are only remotely related to this case. – sixtytrees Jul 21 '16 at 14:39
  • Without more context it is hard to define "such" interfaces and hence I can't provide examples for this. I could provide example of when and why interfaces are useful in general but there are plenty of those on the net already. – Thomas Jul 21 '16 at 14:43
  • Advisor said that author of this code "abstracts methods really well". I guess I will go with "personal preference"/"cannot be answered with the details provided" – sixtytrees Jul 21 '16 at 15:01
  • "Advisor" sounds like this is a test/exercise. If that's the case you might want to add that detail to your question. Also I'd really question the design, i.e. "ValueObject" indicates some kind of value is being handled but the code you posted defines some Swing action as the value that is returned. Without knowing what `ValueObject` is meant to be used for it's hard to tell but I'd doubt a Swing action is a valid value. – Thomas Jul 21 '16 at 15:21
  • @Thomas I probably should use "team lead" instead of "advisor". I am working in a company for a while and this is just a new small task. Before I was dealing with "abstraction free" swing components. This time I first see generics and abstraction in drawing a button that shows a JDialog. This code base was growing since 1998 so it has quite a mixture of styles. – sixtytrees Jul 21 '16 at 15:31
  • Ok in that case I'd just leave it for now unless you're having problems with that code. Understanding a complex and old code base is always a hard task. :) - One last note from a design point of view: "ValueObject" isn't exactly the best name and has as much meaning in itself as "SomethingContainer". ;) – Thomas Jul 21 '16 at 15:42
  • As others already said, there might be not enough information about the context, but from the things visible, it rather looks like the opposite of a good abstraction. Unnecessary wrapping, plus information only recognizable via `instanceof` or type casts (what is the meaning of using `MutableValueObject` instead of `ValueObject` here?) plus hardcoded UI strings. And why does a `private` method need an abstraction? – Holger Jul 21 '16 at 17:06
  • @Holger, I would go with your answer. – sixtytrees Jul 21 '16 at 17:07

1 Answers1

3

There are a lot of problems in this code not to mention it is only overcomplicating the problem. There is a principal called KISS and this kind of code goes straight against it.

Some problems details:

  • Private method abstraction without using anything custom (in the posted code).
  • Anonymus (inner) class usage is not preferred. Some readings here.

Additional bad practices:

  • Should be private String title = "Edit"; changeable? No -> why not static final?
  • dialog.setTitle("Values"); contains a hard-wired String.
  • Names do not indicate what are intended to do "ValueObject", "MutableValueObject"

TL.DR: not a good example of generics. Try to keep source code simple it is for humans to read.

Community
  • 1
  • 1
Hash
  • 4,647
  • 5
  • 21
  • 39