Control coupling
Before answering your question directly, allow me to talk a little bit about the control coupling.
If you have a method that has a parameter that denotes what it has to do and inside you have one if or a switch that based on this parameter decides the desired behavior, even more when a type of behavior has little or no relation to another type of behavior, is because they should be separate methods. This is an example of control coupling.
The control involvement gets even worse when some of the parameters serve only for some of the behaviors while the others serve other behaviors, but they are all in the same method signature. An example of a severe case of control coupling would be this:
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;
}
Note that this method houses and mixes two completely different functionalities that have no sense of being together, which results in confusing code, spaghetti, tied, gambiarrado, difficult to use and difficult to change. Everything would be so much easier if they were separated:
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>";
}
Removing the control involvement
Well, your code has a serious control coupling problem, both in the method alteraLinhaTabela how much in the method dadosDasLinhas. To do so, let’s first separate the features that are mixed:
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);
}
}
Class responsibility
I think it’s somewhat obvious that the functionality that provides the name of a Produto should be in class Produto. It is also obvious that this name should, if possible, be properly formatted so that other classes do not need to keep fixing it. However, that’s not what your code shows:
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
This indicates that the class Produto does not provide its name in a correct format, which forces other classes to have to fix it. Now, this means that the class Produto is not doing its part right and that therefore other classes end up having to fix/fix/correct what this class is not doing right but should know, what is a gambiarra. Therefore, the ideal is that the class Produto provide a method that provides the name in the correct format. The same can be said about the value of the product in the class Tabela2.
In fact, this has everything to do with the principle of inversion of dependency - Your code is depending on the details of the behavior of the methods, not just on calling the methods. From this principle can also be inferred the rule don’t talk to strangers - that is, the method should not call other methods than directly related objects. The best alternative then would be to put this in the class Produto:
public String getNomeLimpo() {
return toString().replace(" ", "").replace("[", "").replace("]", "");
}
And that in class Tabela2:
public String getNomeLimpoProduto() {
return getProduto().getNomeLimpo();
}
public String getValorLimpoProduto() {
return getValorProduto().toString().replace("[", "").replace("]", "");
}
And with that, in the method dadosDasLinhas2 we have:
stmt.setString(4, t2.getNomeLimpoProduto());
stmt.setString(5, t2.getValorLimpoProduto());
Security when using the PreparedStatement
Your code does not deal with the PreparedStatement adequately. In particular, if the code makes an exception, the PreparedStatement can remain open in a zombie state as well as the connection. The ideal way to deal with this problem is to use the Try-with-Resources.
In fact, not only the PreparedStatement, as the Connection must also be in a zombie state, since you do not finish it and do not execute the commit. The connection must be in auto-commit mode (and therefore the commit explicit is unnecessary), but even so, it must be properly closed. This must be the flaw you’re reporting which causes nothing to appear in the database.
We can also move Sqls to static fields.
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());
}
Connection management
I assumed your method getConexao() return a newly opened connection in autocommit mode (which is the default) and should be closed after being used (connect, do something, disconnect). I strongly recommend you do this if you’re not doing it.
If this is not the case and you want to insist on reusing a connection, then the beginning of the blocks try would be so:
try (PreparedStatement stmt = getConexao().prepareStatement(ALTERA_LINHA_TABELA1_SQL)) {
However, dealing properly with previously open connections is not a very easy thing to do. The only thing that’s easy in this case is the possibility of screwing up. Seeing the maturity level of the rest of your code, I doubt you have reusable connections implemented correctly, because if you were not having the problem you are having (sorry for the sincerity).
If you were to use previously opened connections in your method getConexao(), you will probably want to have well delimited transactions, and with that would have auto-commit disabled and the commit() and the rollback() explicit somewhere in your code. In addition, you would have to worry about delimiting the scope of the transaction, which usually corresponds to a request in the case of a web system or a action in the case of a desktop system. With this, to deal correctly with these concepts, you have to know how to deal with ThreadLocals and clear the connection if the thread in question can be reused (typical case of web applications). Another and more complicated problem would be dealing with pool connections.
Therefore, I recommend that your method getConexao() return connections that have just been opened, with the autocommit enabled and that are always used with the Try-with-Resources to ensure proper closure. I suggest trying more complex connection management only when you are already a Jedi Knight in Java, because this is not something for Padawans.
Other possible improvements
There are probably more improvements that would be possible, but that depends on more information about your project that is not in your question. In particular I believe that improvements still fit in the following areas:
Delay is spelled "s", not "z"!
The value of the product should be something coming from within the product and not from Tabela2.
The fact of Produto and ValorProduto have methods toString() that produce results that need to be repaired later is somewhat suspicious.
I don’t think the if (t1.getPagamentoTotal() == 0) { ... } is in the most suitable place.
Utilise catch (SQLException e) { throw new RuntimeException(e); } probably not a good way to treat mistakes.
What’s in your method
getConexao()? I have a half written answer, but I cannot finish it without knowing how this method is implemented.– Victor Stafusa