patternjavaMinor
SQLite UPDATEs for persons and a subclass
Viewed 0 times
sqlitepersonssubclassforandupdates
Problem
I am using Java with SQLite and I am wondering if I am in the right track onto writing good code i.e. more or less reusable:
SQL scheme:
```
final static String UPDATE = "UPDATE ";
private static void alterarWrapper(String query){
Connection connection = null;
try
{
// create a database connection
//connection = DriverManager.getConnection("jdbc:sqlite:4rodas.db");
connection = DriverManager.getConnection("jdbc:sqlite:C:/dropbox/4rodas.db");
Statement statement = connection.createStatement();
statement.setQueryTimeout(30);
statement.executeUpdate(UPDATE + query);
}
catch(SQLException e)
{
System.err.println(e.getMessage());
}
finally
{
try
{
if(connection != null)
connection.close();
}
catch(SQLException e)
{
System.err.println(e);
}
}
}
public static void atualizar(Mecanico mecanico){
String pessoa = "Pessoa SET telefone="+mecanico.getTelefone()+
",celular="+mecanico.getCelular()+",nome='"+mecanico.getNome()+
" where CPF='"+mecanico.getCPF()+"';";
System.out.println(pessoa);
alterarWrapper(pessoa);
String apelido = "Mecanico SET apelido='"+mecanico.getApelido()+
"' where Pessoa_CPF='"+ mecanico.getCPF()+"';";
System.out.println(apelido);
alterarWrapper(apelido);
}
public static void atualiza
SQL scheme:
CREATE TABLE Pessoa (
CPF VARCHAR(11) NOT NULL ,
telefone INTEGER ,
celular INTEGER ,
nome VARCHAR(255) ,
PRIMARY KEY(CPF));
CREATE TABLE Mecanico (
Pessoa_CPF VARCHAR(11) NOT NULL ,
apelido INTEGER NOT NULL ,
PRIMARY KEY(Pessoa_CPF) ,
FOREIGN KEY(Pessoa_CPF)
REFERENCES Pessoa(CPF)
ON DELETE NO ACTION
ON UPDATE NO ACTION);```
final static String UPDATE = "UPDATE ";
private static void alterarWrapper(String query){
Connection connection = null;
try
{
// create a database connection
//connection = DriverManager.getConnection("jdbc:sqlite:4rodas.db");
connection = DriverManager.getConnection("jdbc:sqlite:C:/dropbox/4rodas.db");
Statement statement = connection.createStatement();
statement.setQueryTimeout(30);
statement.executeUpdate(UPDATE + query);
}
catch(SQLException e)
{
System.err.println(e.getMessage());
}
finally
{
try
{
if(connection != null)
connection.close();
}
catch(SQLException e)
{
System.err.println(e);
}
}
}
public static void atualizar(Mecanico mecanico){
String pessoa = "Pessoa SET telefone="+mecanico.getTelefone()+
",celular="+mecanico.getCelular()+",nome='"+mecanico.getNome()+
" where CPF='"+mecanico.getCPF()+"';";
System.out.println(pessoa);
alterarWrapper(pessoa);
String apelido = "Mecanico SET apelido='"+mecanico.getApelido()+
"' where Pessoa_CPF='"+ mecanico.getCPF()+"';";
System.out.println(apelido);
alterarWrapper(apelido);
}
public static void atualiza
Solution
Schema
I'm glad to see that you defined a foreign key. However, I find it odd that you specified
Your approach to inheritance is closest to Concrete Table Inheritance. I'm not convinced, though, that such a complication is justified. Single Table Inheritance would be much simpler. In fact, I'm not even convinced that only mechanics should be allowed to have nicknames, so perhaps no inheritance at all would be the most appropriate design.
Wrapper
The effort to ensure that the connection gets closed is clumsy. Since Java 7, the preferred way to close a connection is using a try-with-resources block. The
But you really should not be opening a new connection for every query, because there is a large overhead involved in establishing a connection. Once you make the connection explicit, much of the motivation for having any wrapper at all on top of JDBC disappears.
(There is a middle ground: connection pooling. With connection pooling, "closing" the connection doesn't actually close it, but merely releases it into a pool so that it can be reused.)
The
Splitting
You have SQL injection vulnerabilities due to the way you misused the JDBC API. The statements should use placeholders:
To update a
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. Single Table Inheritance would be much simpler. In fact, I'm not even convinced that only mechanics should be allowed to have nicknames, so perhaps no inheritance at all would be the most appropriate design.
Wrapper
The effort to ensure that the connection gets closed is clumsy. Since Java 7, the preferred way to close a connection is using a try-with-resources block. The
Statement should be closed using try-with-resources as well, which you neglected to do.But you really should not be opening a new connection for every query, because there is a large overhead involved in establishing a connection. Once you make the connection explicit, much of the motivation for having any wrapper at all on top of JDBC disappears.
(There is a middle ground: connection pooling. With connection pooling, "closing" the connection doesn't actually close it, but merely releases it into a pool so that it can be reused.)
The
UPDATE queriesSplitting
"UPDATE " into a separate string constant is weird and unhelpful.You have SQL injection vulnerabilities due to the way you misused the JDBC API. The statements should use placeholders:
"UPDATE Pessoa SET telephone=?, celular=?, nome=? WHERE CPF=?"To update a
Mecanico, you need to make updates to two tables. The two updates should probably be grouped into one transaction, so that either both updates succeed or neither update has any effect.Code Snippets
"UPDATE Pessoa SET telephone=?, celular=?, nome=? WHERE CPF=?"Context
StackExchange Code Review Q#97645, answer score: 4
Revisions (0)
No revisions yet.