Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is my first time writing a Swing+Groovy application that successfully inserts data to SQL tables. It may look like example code, but it is more test code, as I will eventually expand this to a much bigger application, and would like to see if I can improve it before expanding it.

What it does when you run it is bring up an input window, like this:

user form

And (for now) after you click submit, it will print this to the console:

You entered: First name: John Last name: Smith Phone: (111) 123-4567 Date of birth: 1980-06-28 Sql Instance: groovy.sql.Sql@1383c410 New data appended to table: Person Id: 1 Name: John Smith Phone: (111) 123-4567 Date of birth: 1980-06-28

How can I improve this?

/**
 * @author github.com/Phrancis
 */
import groovy.sql.Sql
import groovy.swing.SwingBuilder
import java.awt.*

/**
 * SQL queries to be called by the Swing application.
 * @TODO: Extract queries to a separate file
 */
def createTestTable = """
        START TRANSACTION;
        DROP TABLE IF EXISTS test;
        CREATE TABLE test (
            id SERIAL,
            first_name TEXT,
            last_name TEXT,
            phone TEXT,
            date_of_birth DATE
            );
        COMMIT;"""

def insertQuery = """
        START TRANSACTION;
        INSERT INTO test (first_name, last_name, phone, date_of_birth)
            VALUES ( ?, ?, ?, CAST(? AS DATE) );
        COMMIT;"""

/**
 * Define input field variables.
 */
class UserInput {
    String firstName
    String lastName
    String phone
    String dateOfBirth
}

/**
 * Initialize values for input fields.
 */
def userInput = new UserInput(
        firstName: null,
        lastName: null,
        phone: null,
        dateOfBirth: null)

/**
 * Swing application starts here.
 */
def swingBuilder = new SwingBuilder()
swingBuilder.edt {

    // style of form
    lookAndFeel 'nimbus'

    // outer frame size
    def width = 400
    def height = 300

    // outer frame
    frame (title: 'User information',
            size: [width, height],
            show: true,
            locationRelativeTo: null ) {

        borderLayout(vgap: 5)

        // inner panel
        panel(constraints:
                BorderLayout.CENTER,
                border: compoundBorder(
                        [emptyBorder(10),
                         titledBorder('Complete the following form:')])) {
            // input fields
            tableLayout {
                tr {
                    td { label 'First name:' }
                    td { textField id:'firstName', columns: 20 }
                }
                tr {
                    td { label 'Last name:' }
                    td { textField id:'lastName', columns: 20 }
                }
                tr {
                    td { label 'Phone:' }
                    td { textField id:'phone', columns: 20 }
                }
                tr {
                    td { label 'Date of birth:' }
                    td { textField id:'dateOfBirth', columns: 20 }
                }
            }
        }
        /**
         * Confirm user input by printing to console
         * @TODO Include this in the GUI
         */
        panel(constraints: BorderLayout.SOUTH) {
            button text: 'Submit', actionPerformed: {
                println """
You entered:
First name: ${userInput.firstName}
Last name: ${userInput.lastName}
Phone: ${userInput.phone}
Date of birth: ${userInput.dateOfBirth}
                """

                /**
                 * Open DB connection, create a table, insert data.
                 * @TODO Extract credentials to configuration file.
                 * @TODO Extract SQL queries to a separate file
                 */
                def dbUrl      = 'jdbc:postgresql://localhost/GroovyTest'
                def dbUser     = 'Phrancis'
                def dbPassword = 'test'
                def dbDriver   = 'org.postgresql.Driver'

                def sql = Sql.newInstance(dbUrl, dbUser, dbPassword, dbDriver)

                println 'Sql Instance: ' + sql

                sql.execute 'SET SEARCH_PATH TO groovy_test;'

                sql.execute createTestTable

                def personData = [userInput.firstName, userInput.lastName, userInput.phone, userInput.dateOfBirth]

                sql.execute insertQuery, personData

                /**
                 * Confirm that data was inserted successfully.
                 * For admin purposes only, no need to add to GUI.
                 */

                sql.eachRow('''SELECT 1
                                  id
                                , first_name
                                , last_name
                                , phone
                                , date_of_birth
                                FROM test
                                ORDER BY id DESC;''') { row ->
                    println """
New data appended to table:
Person Id: ${row.id}
Name: ${row.first_name} ${row.last_name}
Phone: ${row.phone}
Date of birth: ${row.date_of_birth}"""
                }

                sql.close()

            }
        }
        // Bind the fields to the bean
        bean userInput, firstName: bind { firstName.text }
        bean userInput, lastName: bind { lastName.text }
        bean userInput, phone: bind { phone.text }
        bean userInput, dateOfBirth: bind { dateOfBirth.text }
    }
}
share|improve this question

2 Answers 2

up vote 1 down vote accepted

This drops the table every time you start the application, so previous data is removed,

DROP TABLE IF EXISTS test;

Are you sure this is what you really want?

I hope you know what you're doing here, so that you're aware of that.


def userInput = new UserInput(
        firstName: null,
        lastName: null,
        phone: null,
        dateOfBirth: null)

The fields of UserInput are automatically initialized to null, this can be simply:

def userInput = new UserInput()

You extracted variables/constants for width and height, but not for columns? You have the following lines in your code:

td { textField id:'firstName', columns: 20 }
td { textField id:'lastName', columns: 20 }
td { textField id:'phone', columns: 20 }
td { textField id:'dateOfBirth', columns: 20 }

In case you'd ever want to change the columns on all at once, I'd recommend extracting a constant for that.


This looks slightly ugly:

bean userInput, firstName: bind { firstName.text }
bean userInput, lastName: bind { lastName.text }
bean userInput, phone: bind { phone.text }
bean userInput, dateOfBirth: bind { dateOfBirth.text }

It can be written as:

bean userInput,
    firstName: bind { firstName.text },
    lastName: bind { lastName.text },
    phone: bind { phone.text },
    dateOfBirth: bind { dateOfBirth.text }

Overall, very nicely done. I'd recommend you to implement the TODO things, and then get back for another review.

share|improve this answer

Rather than run a query for every insert and then select you just inserted back out to verify that everything worked correctly (doesn't your database interface return the number of rows modified?), take advantage of features in the database like returning.

INSERT INTO test
    (first_name, last_name, phone, date_of_birth)
VALUES
    (?, ?, ?, ? :: date)
RETURNING
    *
    -- alternately...
    -- RETURNING id, first_name, last_name, phone, date_of_birth
;
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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