HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

GUI SQL interface for select insert and remove

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
insertsqlremoveinterfaceforandselectgui

Problem

This is my final project for my second Java class! I would like to get some feedback on it to see where and what I can improve.

There are five class files:

-
GuiMusic

This contains all all of the the Swing and AWT components and the key listeners.

-
ScrollPaneSync

Synchronizes the scroll bars on each JPanel.

-
Stringbillder

Manipulates result set data into a different form so that it can be set onto
JTextFields.

-
SqlCon

Contains the connection and the methods for sending queries.

-
UpdateQuery

This is another data manipulation class used for insert and delete queries.

GuiMusic

I'm open to any critisim such that it's constructive but I have some specific questions

-
How would you split the class to make it smaller?

-
A lot of the ActionListeners have very similiar functionality, is it better to reuse them with different sources and expand the class or should everything have its own listener?

I avoided some code repetition but I'm not sure that that's a good thing when you it might slow your program down.

-
Could generics improve the flexibility of my code?

```
package guiMusic;

public class GuiMusic extends JFrame
{

private static final long serialVersionUID = 1L;
/**
* @conn
* object represents the connection to the database
*/
SqlCon conn;
/**
* @stmt
* used by conn to process sql query results
*/
Statement stmt;
ResultSet rs;
/**
* @sb
* StringBuilder used to manipulate query data
*/
StringBillder sb;
/**
* @formatedData
* used to store information from textAreas and pass to conn
*/
StringBuilder[] formatedData;
/**
* @addInfo
* after a query from the add tab things are the information is stored in addinfo
*/
String[] addInfo;
/**
* @sync
* class that synchronizes scrollbars together
*/
ScrollPaneSync sync;
/**
* @query
* pretty self explanatory
*/

Solution

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 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) {
    // ...
}


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 list = new ArrayList<>(), instead of ArrayList list = .... This is because users of the list instance only needs to understand they are dealing with the List interface.

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. :)

Code Snippets

addClear.setFont(tmu);
addClear.setPreferredSize(new Dimension(220,60));
addClear.setName("addClear");
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;
}
// note: capital 'B' for ButtonListener
setup(addClear, "addClear", tmu, 220, 60).addActionListener(new ButtonListener());
for (JTextField textField : searchLabel) {
    // ...
}
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());
    }

Context

StackExchange Code Review Q#99030, answer score: 2

Revisions (0)

No revisions yet.