Acoplamento de controle
Antes de responder a sua dúvida diretamente, permita-me falar um pouco sobre o acoplamento de controle.
Se você tem um método que tem um parâmetro que denota o que ele tem que fazer e lá dentro você tem um if
ou um switch
que com base neste parâmetro decide o comportamento desejado, ainda mais quando um tipo de comportamento tem pouca ou nenhuma relação com um outro tipo de comportamento, é porque deveriam ser métodos separados. Isso daí é um exemplo de acoplamento de controle.
O acomplamento de controle fica ainda pior quando alguns dos parâmetros servem só para alguns dos comportamentos enquanto os outros servem para outros comportamentos, mas todos eles estão na mesma assinatura de método. Um exemplo de um caso grave de acoplamento de controle seria isso:
public String desenharCirculoOuEscreverXML(int oQueFazer, Graphics g, Produto p) {
if (oQueFazer == 1) {
g.setColor(Color.RED);
g.drawEllipse(20, 20, 50, 50);
} else if (oQueFazer == 2) {
return "<produto><nome>" + p.getNome() + "</nome><codigo>" + p.getCodigo() + "</codigo></produto>";
}
return null;
}
Observe que este método abriga e mistura duas funcionalidades completamente diferentes que não tem sentido de estarem juntas, o que resulta em código confuso, espaguete, amarrado, gambiarrado, difícil de se usar e difícil de se alterar. Tudo ficaria muito mais fácil se elas fossem separadas:
public void desenharCirculo(Graphics g) {
g.setColor(Color.RED);
g.drawEllipse(20, 20, 50, 50);
}
public String escreverXML(Produto p) {
return "<produto><nome>" + p.getNome() + "</nome><codigo>" + p.getCodigo() + "</codigo></produto>";
}
Removendo o acomplamento de controle
Pois bem, o seu código tem um sério problema de acoplamento de controle, tanto no método alteraLinhaTabela
quanto no método dadosDasLinhas
. Para tanto, vamos primeiro separar as funcionalidades que estão misturadas:
public void alteraLinhaTabela1(Tabela1 t1) {
if (t1.getPagamentoTotal() == 0) {
deleteLinha(t1.getId(), 1);
insereLinhaTabela3(t1);
return;
}
try {
String query = "update tabela1 set nome=?, dataPagamento=?, atrazo=?, pagamentoTotal=? where id=?";
PreparedStatement stmt = getConexao().prepareStatement(query);
dadosDasLinhas1(stmt, t1);
stmt.setInt(5, t1.getId());
stmt.execute();
stmt.close();
} catch(SQLException e) {
throw new RuntimeException(e);
}
}
public void alteraLinhaTabela2(Tabela2 t2) {
try {
String query = "update tabela2 set endereco=?, telefone=?, cpf=?, produto=?, valorProduto=? where id=?";
PreparedStatement stmt = getConexao().prepareStatement(query);
dadosDasLinhas2(stmt, t2);
stmt.setInt(6, t2.getId());
stmt.execute();
stmt.close();
} catch(SQLException e) {
throw new RuntimeException(e);
}
}
private void dadosDasLinhas1(PreparedStatement stmt, Tabela1 t1) {
try {
stmt.setString(1, t1.getNome());
stmt.setString(2, t1.getDataPagamento());
stmt.setInt(3, t1.getAtrazo());
stmt.setDouble(4, t1.getPagamentoTotal());
} catch(SQLException e) {
throw new RuntimeException(e);
}
}
private void dadosDasLinhas2(PreparedStatement stmt, Tabela2 t2) {
try {
stmt.setString(1, t2.getEndereco());
stmt.setString(2, t2.getTelefone());
stmt.setString(3, t2.getCpf());
stmt.setString(4, t2.getProduto().toString().replace(" ", "")//retira espaços
.replace("[", "").replace("]", ""));//retira cochetes
stmt.setString(5, t2.getValorProduto().toString().replace(" ", "")//retira espaços
.replace("[", "").replace("]", ""));//retira cochetes
} catch(SQLException e) {
throw new RuntimeException(e);
}
}
Responsabilidade das classes
Acho que é um tanto óbvio que a funcionalidade que fornece o nome de um Produto
deveria estar na classe Produto
. Também é óbvio que esse nome deveria, se possível, vir adequadamente formatado para que outras classes não precisem ficar consertando-o. Entretanto, não é isso que o seu código mostra:
stmt.setString(4, t2.getProduto().toString().replace(" ", "")//retira espaços
.replace("[", "").replace("]", ""));//retira cochetes
stmt.setString(5, t2.getValorProduto().toString().replace(" ", "")//retira espaços
.replace("[", "").replace("]", ""));//retira cochetes
Isso indica que a classe Produto
não fornece o seu nome em um formato correto, o que obriga outras classes a terem que consertá-lo. Ora, isso significa que a classe Produto
não está fazendo a sua parte direito e que portanto outras classes acabam tendo que arrumar/consertar/corrigir aquilo que esta classe não está sabendo fazer direito mas que deveria saber, o que é uma gambiarra. Assim sendo, o ideal é que a classe Produto
disponibilize um método que forneça o nome no formato correto. O mesmo pode ser dito sobre o valor do produto na classe Tabela2
.
Aliás, isso tem tudo a ver com o princípio da inversão de dependência - O seu código está dependendo de detalhes do comportamento dos métodos, e não apenas em chamar os métodos. Desse princípio também pode ser inferida a regra não fale com estranhos - ou seja, o método não deve chamar outros métodos que não sejam de objetos diretamente relacionados. A melhor alternativa então seria colocar isso na classe Produto
:
public String getNomeLimpo() {
return toString().replace(" ", "").replace("[", "").replace("]", "");
}
E isso na classe Tabela2
:
public String getNomeLimpoProduto() {
return getProduto().getNomeLimpo();
}
public String getValorLimpoProduto() {
return getValorProduto().toString().replace("[", "").replace("]", "");
}
E com isso, no método dadosDasLinhas2
temos:
stmt.setString(4, t2.getNomeLimpoProduto());
stmt.setString(5, t2.getValorLimpoProduto());
Segurança ao usar o PreparedStatement
O seu código não lida com o PreparedStatement
adequadamente. Em especial, se o código lançar uma exceção, o PreparedStatement
poderá permanecer aberto em um estado zumbi, assim como a conexão também. A forma ideal de lidar com este problema é usar o try-with-resources.
Aliás, não só o PreparedStatement
, como a Connection
também deve estar em um estado zumbi, uma vez que você não a finaliza e nem executa o commit. A conexão deve estar em modo auto-commit (e portanto o commit
explícito é desnecessário), mas mesmo assim, ela deve ser adequadamente fechada. Esta deve ser a falha que você está relatando que faz com que nada apareça no banco de dados.
Também podemos mover as SQLs para campos estáticos.
private static final String ALTERA_LINHA_TABELA1_SQL =
"UPDATE tabela1 SET nome = ?, dataPagamento = ?, atrazo = ?, pagamentoTotal = ? WHERE id = ?";
private static final String ALTERA_LINHA_TABELA2_SQL =
"UPDATE tabela2 SET endereco = ?, telefone = ?, cpf = ?, produto = ?, valorProduto = ? where id = ?";
public void alteraLinhaTabela1(Tabela1 t1) {
if (t1.getPagamentoTotal() == 0) {
deleteLinha(t1.getId(), 1);
insereLinhaTabela3(t1);
return;
}
try (
Connection c = getConexao();
PreparedStatement stmt = c.prepareStatement(ALTERA_LINHA_TABELA1_SQL);
) {
dadosDasLinhas1(stmt, t1);
stmt.setInt(5, t1.getId());
stmt.execute();
} catch (SQLException e) {
throw new RuntimeException(e);
}
}
public void alteraLinhaTabela2(Tabela2 t2) {
try (
Connection c = getConexao();
PreparedStatement stmt = c.prepareStatement(ALTERA_LINHA_TABELA1_SQL);
) {
dadosDasLinhas2(stmt, t2);
stmt.setInt(6, t2.getId());
stmt.execute();
} catch (SQLException e) {
throw new RuntimeException(e);
}
}
private void dadosDasLinhas1(PreparedStatement stmt, Tabela1 t1) throws SQLException {
stmt.setString(1, t1.getNome());
stmt.setString(2, t1.getDataPagamento());
stmt.setInt(3, t1.getAtrazo());
stmt.setDouble(4, t1.getPagamentoTotal());
}
private void dadosDasLinhas2(PreparedStatement stmt, Tabela2 t2) throws SQLException {
stmt.setString(1, t2.getEndereco());
stmt.setString(2, t2.getTelefone());
stmt.setString(3, t2.getCpf());
stmt.setString(4, t2.getNomeLimpoProduto());
stmt.setString(5, t2.getValorLimpoProduto());
}
Gerenciamento de conexões
Eu parti da suposição que o seu método getConexao()
devolva uma conexão recém-aberta, no modo autocommit (que é o padrão) e que deveria ser fechada em seguida após ser utilizada (conecta, faz alguma coisa, desconecta). Recomendo fortemente que faça isso caso não esteja fazendo.
Se não for este o caso e você quiser insistir em reutilizar uma conexão, então o início dos blocos try
seria assim:
try (PreparedStatement stmt = getConexao().prepareStatement(ALTERA_LINHA_TABELA1_SQL)) {
Entretanto, lidar adequadamente com conexões já abertas anteriormente não é algo muito fácil de se fazer. A única coisa que é fácil nesse caso é a possibilidade de se fazer besteira. Vendo o nível de maturidade do resto do seu código, duvido que você tenha conexões reutilizáveis implementadas de forma correta, pois se estivesse não estaria tendo o problema que está tendo (desculpe-me a sinceridade).
Caso você fosse utilizar conexões já abertas anteriormente no seu método getConexao()
, você provavelmente vai querer ter transações bem delimitadas, e com isso teria o auto-commit desabilitado e o commit()
e o rollback()
explícitos em algum lugar do seu código. Além disso, você teria que se preocupar em delimitar o escopo da transação, que normalmente corresponde a uma requisição no caso de um sistema web ou de uma action no caso de um sistema desktop. Com isso, para lidar corretamente com esses conceitos, você tem que saber lidar com ThreadLocal
s e limpar a conexão caso a thread em questão puder ser reutilizada (caso típico de aplicações web). Um outro problema além disso e mais complicado seria lidar com conexões em pool.
Assim sendo, recomendo que o seu método getConexao()
devolva conexões que tenham acabado de ser abertas, com o autocommit habilitado e que sejam sempre utilizadas com o try-with-resources para garantir o fechamento adequado. Sugiro tentar um gerenciamento de conexões mais complexo apenas quando você já for um Cavaleiro Jedi no Java, porque isso não é coisa para padawans.
Outras possíveis melhorias
Provavelmente há mais melhorias que seriam possíveis, mas isso depende de mais informações sobre o seu projeto que não estão na sua pergunta. Em especial acredito que melhorias ainda cabem nas seguintes áreas:
Atraso se escreve com "s", e não com "z"!
O valor do produto deveria ser algo vindo de dentro do produto e não de Tabela2
.
O fato de Produto
e ValorProduto
terem métodos toString()
que produzem resultados que necessitam ser consertados depois é um tanto suspeito.
Não acho que o if (t1.getPagamentoTotal() == 0) { ... }
esteja no lugar mais adequado.
Utilizar catch (SQLException e) { throw new RuntimeException(e); }
provavelmente não é uma boa forma de se tratar erros.
getConexao()
? Estou com uma resposta escrita pela metade, mas não consigo terminá-la sem saber como este método está implementado.