Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/

edited Apr 13, 2017 at 12:40

Is the misspelling StringBillder intentional so that it doesn't conflict with that of StringBuilder? I think you need a much better class name here... - myself myself

Is the misspelling StringBillder intentional so that it doesn't conflict with that of StringBuilder? I think you need a much better class name here... - myself

added 410 characters in body

Source Link

edited Sep 22, 2015 at 16:21

h.j.k.

edited Sep 22, 2015 at 16:21

h.j.k.

19.3k
3
37
93

edit: ah, so you did use these in your poorly-named StringBillder, but I'm not too sure how that is being used in the first place, and whether it is appropriate. For starters, you should declare your variables as List<T> list = new ArrayList<>(), instead of ArrayList<T> list = .... This is because users of the list instance only needs to understand they are dealing with the List interface.

Source Link

answered Sep 22, 2015 at 16:09

h.j.k.

answered Sep 22, 2015 at 16:09

h.j.k.

19.3k
3
37
93

Code duplication

You have a lot of similar code blocks that set the following properties:

addClear.setFont(tmu);
addClear.setPreferredSize(new Dimension(220,60));
addClear.setName("addClear");

You can consider extracting these out to a method that you can reuse:

private static <T extends JComponent> T setup(T component, String name, Font font, 
 int preferredWidth, int preferredHeight) {
 component.setName(name);
 component.setFont(font);
 component.setPreferredSize(new Dimension(preferredWidth, preferredHeight));
 return component;
}

The idea of return-ing the component is to let you daisy-chain this method with calling other properties of the component itself, e.g.

// note: capital 'B' for ButtonListener
setup(addClear, "addClear", tmu, 220, 60).addActionListener(new ButtonListener());

Collections vs arrays, and iterating through them

You seem to rely a lot on plain arrays... but I think one should tend towards the Collection classes as they are more descriptive in what their identity, e.g. a Set tells you that elements are distinct, and a List implies an ordering. Regardless of which you choose, you can already use the enhanced for-each loop (since Java 1.5) instead of the index-based for-loop, especially since you do not use the index:

for (JTextField textField : searchLabel) {
 // ...
}

Bracing style

The good news here is that your bracing style, while non-Java-conventional, is largely consistent. The only two exceptions I see are for your catch statements in try-catch blocks and the final else/else if clauses, so you may wish to improve on that.

Oh, and hang on for a moment, inside textFieldListener.keyReleased(KeyEvent)...

 else if(remove);
 {
 remText[0].setText(formatedData[0].toString());
 remText[1].setText(formatedData[1].toString());
 remText[2].setText(formatedData[2].toString());
 remText[3].setText(formatedData[3].toString());
 }

There is a bug here, I'll let you figure it out ;).

Names

Your *Listener implementations should be using PascalCase for the class name too. Your arrays are better off in the plural forms, as it reads (or sounds) better that you are iterating through searchLabels than (a) searchLabel.

Is the misspelling StringBillder intentional so that it doesn't conflict with that of StringBuilder? I think you need a much better class name here... - myself

And yeah, this earlier comment applies too.

SQL and prepared statements

Initially, I was wondering why does your query template/patterns have so many %-s in them, before I realized they are used for String.format(String, Object...) as placeholders and to escape the literal % character.

The thing is, prepared statements for SQL queries are the way to go, and we don't mean literally preparing them using String.format(). I'll suggest reading the documentation here for more information on this topic. :)

default