Suggestions/Improvements for Database class?

Discussion in 'Spigot Plugin Development' started by EvanTheSurfer, Jun 5, 2017.

  1. Hi. Ive been learning mysql, and have constructed this class below. I have read that establishing a connection will lag the main thread, so I have made it establish in a seperate thread. My methods are made thread safe because I will be using getConnection() in multiple other threads to retrieve and set info in my db. If there is anything i need to improve on please let me know, as Id rather not be using non efficient code and I am a beginner with dbs.

    Code (Text):
    public class Database {

        private String host, name, username, password;
        private final int port = 3306;
        private Connection connection;

        private Database() {
            // Prevent Instantiation
        }

        public Database(String host, String name, String username, String password) {
            this.host = host;
            this.name = name;
            this.username = username;
            this.password = password;
        }

        public void establishConnection() {
            Thread thread;
            try {
                if (connection != null && !connection.isClosed()) {
                    return;
                }
                thread = new Thread(new Runnable() {
                    @Override
                    public void run() {
                        try {
                            connection = (Connection) DriverManager
                                    .getConnection("jdbc:mysql://" + host + ":" + port + "/" + name, username, password);
                        } catch (SQLException e) {
                            e.printStackTrace();
                        }
                    }
                });
                thread.start();
            } catch (SQLException e) {
                 e.printStackTrace();
            }
        }

        public synchronized Connection getConnection() {
            if (connection == null) {
                this.establishConnection();
                return connection;
            }
            return connection;
       
        }

    }
     
    EDIT: Removed thread safety. Decided to not use a single connection object.
     
    #1 EvanTheSurfer, Jun 5, 2017
    Last edited: Jun 5, 2017
  2. Are you using batch processing where appropriate?
    What are your statements, can they be optimized?
    Are you closing everything properly in a finally block to prevent database lock?

    That's what comes to mind.
     
  3. You should set connection timeout.
     
  4. Remove the empty constructor, it serves no purpose. Also, using Bukkit async tasks is more efficient than starting your own threads (there's a possibility it'll reuse an existing thread, saving you the overhead)
     
  5. You should look into using a connection pool (I recommend HikariCP)
     
    • Like Like x 1
  6. I have not added those methods yet. I just wanted to know if the construction of my database was correct or sufficient. I did add one method that updates, which I did indeed close the prepared statement, and the connection. I am thinking about looking into cp as @Redrield said, as it reuses connections from a pool I think instead of creating a new one each time. I may look into that.
    I actually never knew that starting an async task using bukkit would do that. I'll change that and remove the private constructor.

    I read on a forum somewhere that a connection Default unused time is 30 secs. But that doesn't sound quite right. Can someone clarify?