4
\$\begingroup\$

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:

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 atualizar(Pessoa cliente){
          String pessoa = "Pessoa SET telefone="
                  +cliente.getTelefone()+",celular="+cliente.getCelular()+",nome="+cliente.getNome()+" where CPF='"+ cliente.getCPF() +"';";
          alterarWrapper(pessoa);
      }

My doubts are:

  1. About the update methods are they are updating everything but the primary key, because I cannot really see how to implement a way of updating just the necessary.
  2. Is it a good idea to do atualizar(Class class) and then format and use the wrapper?
\$\endgroup\$
4
\$\begingroup\$

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. 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 queries

Splitting "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.

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

Apart from the SQL injection issue mentioned by 200_success I'd use String.format() instead of string concatenation in general.

E.g.:

String pessoa = String.format(
                  "Pessoa SET telefone=%d,celular=%d,nome='%s' where CPF='%s';",
                  mecanico.getTelefone(),
                  mecanico.getCelular(),
                  mecanico.getNome(),
                  mecanico.getCPF());

instead of:

String pessoa = "Pessoa SET telefone="+mecanico.getTelefone()+
              ",celular="+mecanico.getCelular()+",nome='"+mecanico.getNome()+
              " where CPF='"+mecanico.getCPF()+"';";

It's not only clearer. Such you probably would have recognized easier that there is a "'" missing at nome='...' in your code.

\$\endgroup\$

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.