7
\$\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\$

3 Answers 3

7
\$\begingroup\$

General Approach

I generally do not like wrapper classes like these.

They seldom 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-known 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\$
0
4
\$\begingroup\$

Improvements

  • You are using the C style array notation. This was added in the original java to be compatible with C and C++.

      String fields[]  // Unusual C style.
      String[] fields  // More logical: type parts together
    
  • Avoid boolean-on-boolean expressions

      X == true            X == false       // Redundant
      X                    !X               // Straight logic
    

Wrapping database calls in one's own class is very alluring, often seen, but there are good reasons to do other things in production code. Libraries, but also using PreparedStatement which prevents malicious SQL injection and leaving it to the driver escaping special characters (like apostrophe). And being type-safe.

Statement/PreparedStatement, ResultSet, Connection come all with a .close() ( AutoCloseable interface).

Pass the exceptions up: ...) throws SQLException {. However some errors like wrong SQL or wrong column name could be captured and rethrown as respectively throw new IllegalStateException(sql) or throw new IllegalArgumentException("Wrong column name in " + fields, e). Whereas SQLException and IOException are checked exception you are forced to catch, the two above are RuntimeException.

\$\endgroup\$
0
\$\begingroup\$

While mentioning it should not be done vanishes any chance to not do it I mention do not do it since my opinion it is that all a wrapper of a standard can do is to decrease efficiency without adding value, it is just a third party imposed between a standard and the users of the standard, of course if the users agree to use the third party library in spite of the standard.

If for some random reason(s) there are insistence on being implemented, there are two concerns to address: (1) the build of sql statements and (2) the use of the built statements therefore there should be an sql statement builder and data access objects (DAOs) using the builder. The sql statement builder sole concern should be to build sql statements with proper sql syntax. The DAOs concern should be to sanitise any value set in the statement, create the query (prepared statement or plain statement), run it and parse the result set. The exceptions thrown while any step of a DAO method should be let bubble to the layer calling the DAO methods since the method could be part of a transaction (that is a sac connection with autocommit set to false) that have to be aware of the success/failure of each query being part of it to be able to rollback in case of failure.

Since the DAO methods are classical implementations following is a depict of an SQL statement builder implementation. The test class...

public class SqlStatementTest {

    public static void main(String[] args) {
        String sql = PlainSqlBuilder.select("column")
                                    .from("tableName")
                                    .where("column = value")
                                    .groupBy("grouped_column")
                                    .having("grouped_column > 1")
                                    .and("grouped_column < 10")
                                    .orderBy("order_column")
                                    .sql();
        System.out.println(sql);

        System.out.println(PlainSqlBuilder.select("column").sql());
        
        System.out.println(PlainSqlBuilder.select("column")
                                          .from("table")
                                          .sql());

        System.out.println(PlainSqlBuilder.select("t.column")
                                          .from("table t")
                                          .orderBy("t.column DESC")
                                          .sql());

    }
}

...outputs...

SELECT column FROM tableName WHERE column = value GROUP BY grouped_column HAVING grouped_column > 1 AND ggrouped_column < 10 ORDER BY order_column

SELECT column

SELECT column FROM table

SELECT t.column FROM table t ORDER BY t.column DESC

...with the sql builder possible implementation consisting of an interface declaring a sole method used to describe parts of the sql statements...

interface Builder {

    String sql();
}

...and its implementations encapsulated into a builder with fluent interface design pattern implementation...


import java.util.List;
import java.util.stream.Stream;

public class PlainSqlBuilder {
    
    public static SelectBuilder select(String... columns) {
        return SelectBuilder.columns(columns);
    }
    
    private PlainSqlBuilder() {}
    
    public static class SelectBuilder implements Builder  {
        public static SelectBuilder columns(String... names) {
            return new SelectBuilder(names);
        }
        
        private enum SqlClause {
            WHERE("WHERE"), HAVING("HAVING");
            
            public String keyword;
            
            private SqlClause(String keyword) {
                this.keyword = keyword;
            }
        }
        
        public static class LogicOperator implements Builder {
            
            private Builder prefix;
            private StringBuilder conditions;
            
            private LogicOperator(Builder prefix
                                , SqlClause clause
                                , String condition) {
                this.conditions = new StringBuilder(" ").append(clause.keyword)
                                                        .append(" ")
                                                        .append(condition);
                this.prefix = prefix;
            }

            public LogicOperator or(String condition) {
                this.conditions.append(" OR ").append(condition);
                return this;
            }
            
            public LogicOperator and(String condition) {
                this.conditions.append(" AND ").append(condition);
                return this;
            }
            public LogicOperator or(String... conditions) {

                Stream.of(conditions).forEach(condition -> this.conditions
                                                               .append(" OR ")
                                                               .append(condition));
                return this;
            }
            
            public LogicOperator and(String... conditions) {
                
                Stream.of(conditions).forEach(condition -> this.conditions
                                                               .append(" AND ")
                                                               .append(condition));
                return this;
            }
            
            public GroupByBuilder groupBy(String column) {
                return new GroupByBuilder(this, column);
            }

            public OrderByBuilder orderBy(String... columns) {
                return new OrderByBuilder(this, columns);
            }
            
            public String sql() {
                return this.prefix.sql() + this.conditions.toString();
            }
        }
        
        public static class OrderByBuilder implements Builder {
            private Builder prefix;
            private List<String> columns;
            
            private OrderByBuilder(Builder prefix, String... columns) {
                this.prefix = prefix;
                this.columns = List.of(columns);
            }

            public String sql() {
                return this.prefix.sql() + " ORDER BY "
                                         + String.join(", ", this.columns);
            }
        }
        
        public static class GroupByBuilder implements Builder {
            
            private Builder prefix;
            private String column;
            
            private GroupByBuilder(Builder prefix, String column) {
                this.prefix = prefix;
                this.column = column;
            }
            
            public LogicOperator having(String condition) {

                return new LogicOperator(this, SqlClause.HAVING, condition);
            }
            
            public OrderByBuilder orderBy(String column) {
                return new OrderByBuilder(this, column);
            }
            
            public String sql() {
                return this.prefix.sql() + " GROUP BY " + this.column;
            }
        }
        
        public static class SqlClauseBuilder implements Builder {
            
            private Builder prefix;
            
            private SqlClauseBuilder(Builder prefix) {
                this.prefix = prefix;
            }

            public LogicOperator where(String condition) {
                return new LogicOperator(this.prefix, SqlClause.WHERE, condition);
            }
            
            public GroupByBuilder groupBy(String column) {
                return new GroupByBuilder(this.prefix, column);
            }
            
            public OrderByBuilder orderBy(String column) {
                return new OrderByBuilder(this.prefix, column);
            }

            @Override
            public String sql() {
                
                return this.prefix.sql();
            }
        }
        
        private String[] names;
        private String tableName;
        
        private SelectBuilder(String[] names) {
            this.names = names;
        }
        
        public SqlClauseBuilder from(String tableName) {
            this.tableName = tableName;
            return new SqlClauseBuilder(this);
        }
        private String names() {
            return names == null 
                   || names.length == 0 ? "*"
                                        : String.join(", ", names);
        }
        
        public String sql() {
            StringBuilder sql = new StringBuilder("SELECT ").append(this.names());
            if (tableName != null) {
                sql.append(" FROM ").append(this.tableName);
            }
            return sql.toString();
        }
    }
}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ I'm struggling to make sense of your first sentence. Is there any chance you could reword it, probably breaking it up into smaller sentences? \$\endgroup\$ Commented Mar 11 at 9:58

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.