The Stack Overflow podcast is back! Listen to an interview with our new CEO.

Hot answers tagged

10

Well, you could just call the toArray method of your list to transform the list into an array. Or even better, you could make the table model use a List<Object[]> rather than an Object[][], which would avoid the unnecessary conversion.


9

I have to agree with Praveen's comment that you should open a connection when you need it and close it when your are done with the Database operations. But first I have to make a serious objection to this statement: ps = conn.prepareStatement(SQL_QUERY.replace("?", id)); That is not the way you should use prepared statements! By using that, you are ...


8

A few random notes: If I were you I'd check the source and API of Apache Commons Pool. You could borrow ideas about possible errors, corner cases, useful API calls, parameters etc. surrenderConnection should rollback unfinished transactions. They could leak. SLF4J has better API than JUL. (Check Logback also, if you don't familiar with it.) A malicious (or ...


8

Minor detail: I prefer putting a closing parenthesis on its own line when the content between parentheses spans more than one line. (Basically, it looks like one-true-brace style with parentheses instead of braces.) I find it makes scanning vertically much simpler. If your team/company has chosen a different convention, that's fine. The biggest problem I ...


7

It looks like the borrow connection code will create a new connection without first checking to see if there is an available connection in the pool. That's not how most connection pools work. You probably want to store a count of available connections (consider using an AtomicInteger for safe concurrent access) and check against that before adding the new ...


7

The best way to close a Connection is... not doing it yourself, but using try-with-resources, which is the best practice as of Java 7: public String getStory(String id) throws Exception { try (Connection conn = MysqlDb.getConn(dbName)) { for (Publisher p : Publishers) { PreparedStatement prpStmt = conn.prepareStatement("SELECT story ...


6

It looks thread-safe for me, I've not found any concurrency-related issue except that you might want to to use a ThreadLocalRandom or use separate SecureRandom instances with a ThreadLocal for performance reasons. See also. Is SecureRandom thread safe? boolean valid = true; Instead of this local variable you could return immediately if you know the result: ...


6

Overall: Are you sure that you want to keep a static number of connections? Why not keep a minimum set of connections and create new ones when needed? ConnectionPool shouldn't have static fields as signalpillar said. You don't check if the connections have timed out after a period of wait Loggers are nice -> you don't necessary have to remove all useful ...


6

In addition, you should always close your ResultSet and your Statement explicitly. (Or: also use these in a try-with-resources block, and yes, both are AutoCloseable.) The background is, that these objects might be tied to system resources, and there is no guarantee that the database driver automatically closes these dependent objects when you close the ...


5

This way your software is highly coupled. I can see three big responsibilities in it: Get data from db Format data in "TableModel way" Create TableModel Let's create a class to get data. I don't like the idea of let other classes deal with objects from jdbc api, so this class will format data too: public class TableData { public TableContent getData()...


5

I think you want to use transactions. Within a transaction, you can certainly do INSERT + INSERT + SELECT the UserInfoID + the final INSERT. Commit the transaction at the end, after all successful, rollback on any exception. Rather than using the low level database APIs like managing Connection, PreparedStatement, ResultSet, yourself, I strongly recommend ...


5

Why do you need to load driver each time you call makeConnection()? I suggest to do it only once per class loading. Consider a case when makeConnection() will throw an exception every time you call it and bq will be empty. I suggest to throw a special exception in that case.


5

janos recommended the Spring and Apache DbUtils frameworks, I can highly recommend using Hibernate. Hibernate is a very commonly used ORM framework (Object-Relational-Mapping). By using Hibernate and the proper entity classes, you can do all this with only these lines: try { em.getTransaction().begin(); em.persist(user); em.getTransaction()....


5

Funny coincidence, I just created the solution for your problem (but in PHP). The common thing here is the one-time use of a prepared statement, the difference is in the query string and arguments, so what you need is a function that accepts a query string, an argument determining what execution function to use (executeQuery() or executeUpdate()) and the ...


4

Some ideas: Separate the GUI code from the logic. If you stay with your current mixed code fail fast. If conn is null there no sense to open the file or ask the user to choose one. JFileChooser.getSelectedFile() returns a File instance, pass it to FileInputStream directly, don't concatenate strings. File contains the absolute path. It's a good practice to ...


4

Using polymorphism is the way to get an great solution. You are receiving the class in your method just to infer the the return type while you could abstract the idea of conversion and use it to do the inference too. Let's create an interface to convert the result and its implementations: interface ResultTransformer<T> { T transform(ResultSet rs, ...


4

Extract a method. class Task implements Runnable { ... private completeStatement (CallableStatement stmt, Strung userId) { callableStatement.setString(1, String.valueOf(userId)); ... } @Override public void run() { dbConnection = getDbConnection(); callableStatement = dbConnection.prepareCall(...


4

Schema I'm glad to see that you defined a foreign key. However, I find it odd that you specified NO ACTION, since that means that SQLite will not enforce your data integrity. I would expect either RESTRICT or CASCADE. Your approach to inheritance is closest to Concrete Table Inheritance. I'm not convinced, though, that such a complication is justified. ...


4

There is one important thing you should always do if working with a database: You should always close the connection. Therefore I would suggest adding a finally clause to the try..catch and add the following Connection connection = null; try { Class.forName("oracle.jdbc.driver.OracleDriver"); connection = DriverManager.getConnection( ...


4

Prefer interfaces to implementations public static ArrayList<State> findNeighbours(State currentState) { Consider public static List<State> findNeighbours(State currentState) { This way you can change from ArrayList to another implementation at just one place: the initialization. Remember what you know for (int i = 0; i < state....


4

Resource-leak: you neither close the statement nor the result set. Depending on the database driver, this can lead to system resource leaks, blocked OS handles, etc. Always close database objects ASAP (apart from connections, which you can hold for a longer time.) Multiple looping over metadata: you reiterate over result set metadata for each row in the ...


4

Please use best practices in general when dealing with password hashing. If you have to verify the database for your generated salt to be unique, you're using a bad salt generator. Make sure to focus on using a good generator instead. Your hash function also seems weak. Consider using key stretching. This should not be required: while(isFound){ isFound=...


4

You must never store passwords as plain text in the database. Read an article about password hashing to avoid this mistake in the future. Make sure that you have a unique index on the username column. Otherwise it will be possible to create several users with the same username, and with equal or differing passwords.


3

Per the other example - step 1 should be to get out your profiler. In any case - I suspect you would get significantly reduced round trip times by taking advantage of JDBC batching capability. Here's an example: http://www.tutorialspoint.com/jdbc/jdbc-batch-processing.htm (see the second one - using a prepared statement). In short - you create your DELETE ...


3

First of all I would suggest you to look at this library DBCP As for the code you have showed - It will be better to avoid using of static state and behavior in this class. The way to control number of instances of this class can be propagated to the client code using different approaches - injection for instance. As connection data may change better to ...


3

I agree with @JB Nizet, some other notes about the current implementation: Use the List interface as reference type instead of the implementation type. I mean change ArrayList<Object[]> result = ... to List<Object[]> result = ... Type List vs type ArrayList in Java I'd change the indexing in the for loops: for (int i = 0; i < columnCount;...


3

Some JDBC-related notes: use getColumnLabel() instead of getColumnName() using islast() as part of the ResultSet loop is unconventional (and potentially slow, and potentially not supported). The intended use pattern is while res.next(): ( next() returns true when successful). you should have a symmetrical mechanism for accessing both the column names, as ...


3

I'd use a simple List or ArrayList here. Vector is considered obsolete. (Make sure that accessing is properly synchronized if they're used by multiple threads.) printStackTrace() only catch clauses just postpone the errors and usually makes debugging harder. Clients of these class might use these values (or the empty collections) as valid data. You should ...


3

JDBC has always been a PITA when it comes to handling/closing resources. One of the big advancements in the past while has been the advent of ConnectionPools and abstraction layers.... If you are inside a framwork like tomcat, or WebSphere, these are built in. Otherwise you can use things like Apache DBCP or C3PO. For both of these frameworks, when you ...


3

I didn't understand the concept here. How come ConnectionPool is a part of PooledConnection? My understanding of a pool - you a bucket with 100 items, take one, use it and put it back into the pond. I would prefer writing something like: ConnectionPool.getConnection(); ConnectionPool.releaseConnection(Connection c); Where there will be only 1 instance of ...


Only top voted, non community-wiki answers of a minimum length are eligible