4
\$\begingroup\$

I'm currently enrolled in a further education with the topic database administration.

Among other chapters it contains how databases can be accessed from application-software.

Because it lacks programming exercises I have programmed a bit myself. Just for getting familiar with the techniques (mainly Java JDBC) shown.

I finally got the idea for a MySQL wrapper-class which hides away (at least partly) the actual SQL.

Here's what I got so far:

package jdbc;

import java.sql.*;
import static java.lang.System.*;

public class MySqlDb
{
    private Connection connection;
    private Statement statement;

    public MySqlDb(String url, String dbName, String username, String password) 
            throws SQLException {

            connection = DriverManager.getConnection(
                            "jdbc:mysql://" + url + ":3306/" + dbName, 
                             username, 
                             password);      
    }

    public String select(String tableName, String fields[]) {
        return select(tableName, fields, "1 = 1");
    }

    public String select(String tableName, String fields[], String crits) {
        String selectStatement = "SELECT * "
                               + "FROM " + tableName + " "
                               + "WHERE " + crits;
        String ret = "";        

        try {
            statement = connection.createStatement();
            ResultSet result = statement.executeQuery(selectStatement);

            while (result.next()) {
                for (String field : fields) {
                    String currentFieldValue = result.getString(field);

                    if (currentFieldValue != null) {
                        ret += result.getString(field) + "\t";
                    }
                }

                ret = ret.substring(0, ret.length() - 1) + "\n";
            }

        } catch (SQLException e) {
            err.println(createSqlExceptionInfo(e));
        } finally {
            resetStatement();
        }

        return ret;
    }

    public void insert(String sqlInsert) {
        try {
            statement = connection.createStatement();

            statement.executeUpdate(sqlInsert);
        } catch (SQLException e) {
            err.println(createSqlExceptionInfo(e));
        } finally {
            resetStatement();
        }
    }

    public void update(String sqlUpdate) {
        try {
            statement = connection.createStatement();

            statement.executeUpdate(sqlUpdate);
        } catch (SQLException e) {
            err.println(createSqlExceptionInfo(e));
        } finally {
            resetStatement();
        }
    }

    public void delete(String sqlDelete) {
        try {
            statement = connection.createStatement();

            statement.executeUpdate(sqlDelete);
        } catch (SQLException e) {
            err.println(createSqlExceptionInfo(e));
        } finally {
            resetStatement();
        }
    }

    public boolean closeConnection() {
        try {
            connection.close();

            return true;
        } catch (SQLException e) {
            err.println(createSqlExceptionInfo(e));
        }

        return false;
    }

    public static String createSqlExceptionInfo(SQLException e) {
        String ret   = "SQL-State:\t"            + e.getSQLState()  + "\n";
        ret         += "SQL error-code:\t"       + e.getErrorCode() + "\n";
        ret         += "Description:\t"          + e.getMessage();

        return ret;
    }

    private void resetStatement() {
        if (statement != null) {
            statement = null;
        }
    }
}

It works quite fine. I used it to insert and query data into / from a playground-database I've made.

Here's my test-class:

package jdbc;

import static java.lang.System.*;
import java.sql.SQLException;

public class Jdbc
{
    public static void main(String[] args)
    {        
        MySqlDb mysql = null;

        try {
            mysql = new MySqlDb("localhost", "seminar_db", "root", "root");   
        } catch (SQLException e) {
            MySqlDb.createSqlExceptionInfo(e);
        }

       if (mysql != null) {

            mysql.insert(
                "INSERT INTO Seminar (thema, beschreibung) " +
                    "VALUES ('Java', "
                        + "'Introduction to Java.')");
            mysql.insert(
                "INSERT INTO Seminar (thema, beschreibung) " +
                    "VALUES ('Java 2', "
                        + "'Advanced Java.')");
            mysql.insert(
                "INSERT INTO Seminar (thema, beschreibung) " +
                    "VALUES ('PHP', "
                        + "'Get started with PHP.')");
            mysql.insert(
                "INSERT INTO Seminar (thema, beschreibung) " +
                    "VALUES ('XML', "
                        + "'XML-Introduction.')");
            mysql.insert(
                "INSERT INTO Seminar (thema, beschreibung) " +
                    "VALUES ('HTML/CSS', "
                        + "'Webdesign course')");
            mysql.insert(
                "INSERT INTO Seminar (thema, beschreibung) " +
                    "VALUES ('JavaScript', "
                        + "'Webprogramming')");
       }

       String[] fields = new String[2];
       fields[0] = "thema";
       fields[1] = "beschreibung";

       out.println(mysql.select("Seminar", fields));

       mysql
        .update(
            "UPDATE Seminar SET beschreibung = 'PHP beginner' WHERE thema = 'PHP'"
        );

       out.println(mysql.select("Seminar", fields, "thema = 'PHP'"));

       if (mysql.closeConnection() == true) {
           out.println("Database connection has been closed.");
       } 
    }
}

The output the test-class:

Java    Introduction to Java.
Java 2  Advanced Java.
PHP Get started with PHP.
XML XML-Introduction.
HTML/CSS    Webdesign course
JavaScript  Webprogramming

PHP PHP beginner

Database connection has been closed.
BUILD SUCCESSFUL (total time: 0 seconds)

I had a college semester and an internship in a firm doing Java but I'm not a real Java programmer.

So I don't know the idioms and patterns of the language.

Therefore my question:

What improvements should I made?

Especially:

I catch the most exceptions within the class itself instead of passing it upwards towards the calling method.

Is my exception-handling done in a correct way? Is it good practice to catch exceptions as early as possible?

Any comments, hints and recommendations appreciated.

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

General Approach

I generally do not like wrapper classes like these.

They seldomly add any real functionality, they often make it impossible to use important functionality (such as prepared statements), they do not result in easier to read code, and they replace a well-know API with an unknown one.

If you do not want to use the API provided by Java, you should carefully consider what value your API actually adds. Something like a querybuilder or ORM may be closer to what you actually want.

SQL Injection

You are vulnerable to SQL injection. There is no way to use prepared statements with your approach, so it is inherently flawed.

Exceptions

As you noted, you catch most of the exceptions yourself. But you don't actually handle them at all, you just print an error message.

That is not the correct way to do it. If you can't handle an exception, throw it until you can handle it.

This is especially true for a general-purpose class like this that should be as reusable as possible.

There may be queries where you don't care if they fail (say you are logging something every x seconds, and it doesn't matter if you miss a couple - or all - entries). There may be cases where you want to take an alternative approach if a query fails (maybe you have a secondary source for the info in the db that you want to try), there may be cases where you want to inform the user (eg when inserting a duplicate seminar), or there may be cases where you simply want to log the error for latter debugging and show a generic error message to the user.

API

Your select function works very different than your insert/update/delete, but there is no reason for that.

Also, insert/update/delete are exactly the same, so why have different methods?

Misc

  • Don't import *, but instead just what you actually need.
  • Why are some of your methods static? This doesn't seem necessary.
  • Why is statement a field? That doesn't seem necessary (and neither does resetStatement).
  • Don't shorten variable names. crits isn't much easier to write than criteria, but much more difficult to read.
\$\endgroup\$
  • \$\begingroup\$ Awesome hints. Thanks a lot for your time writing this answer. I appreciate it much. \$\endgroup\$ – michael.zech Mar 7 '17 at 12:15

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.