Acoplamento e Coesão

Salve a todos.

Estava discutindo um código com um colega. E ele argumenta que o meu código tem alto acoplamento por estar dependendo de mais uma função. A minha opinião é que ao modularizar o código deixo-o mais simples mais fácil de ler de testar de reaproveitar.

[code]public class Transfer {

public static void exportarDadosRetorno(ListaRetorno listaRetorno)
		throws TransferException, ConexaoException {
	try (Connection con = ConexaoCop.iniciarConexao()) {
		inserirDadosRetorno(con, listaRetorno);
	} catch (SQLException e) {
		throw new TransferException(
				"Erro ao acessar banco de dados para exportar Dados ", e);
	}
}

private static void inserirDadosRetorno(Connection con,
		ListaRetorno listaRetorno) throws SQLException {

	Statement stm = con.createStatement();
	stm.executeUpdate("BEGIN");

	String sqlInsertLista = "INSERT INTO tabela*(*, *, *, *, *, *, *,"
			+ "*, *, *, *, *) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";

	PreparedStatement stmtInsertLista = con
			.prepareStatement(sqlInsertLista);

	iterarLista(listaRetorno, stmtInsertLista);

	stm.executeUpdate("COMMIT");
}

private static void iterarLista(ListaRetorno listaRetorno,
		PreparedStatement stmtInsertLista) throws SQLException{
	for (Retorno retorno : listaRetorno.getLista()) {
		try {
			stmtInsertLista.setString(1, retorno.*());
			stmtInsertLista.setTimestamp(2, retorno.*());
			stmtInsertLista.setString(3, retorno.*());
			stmtInsertLista.setString(4, retorno.*());
			stmtInsertLista.setString(5, retorno.*());
			stmtInsertLista.setString(6, retorno.*());
			stmtInsertLista.setString(7, retorno.*());
			stmtInsertLista.setString(8, retorno.*());
			stmtInsertLista.setString(9, retorno.*());
			stmtInsertLista.setString(10, retorno.*());
			stmtInsertLista.setString(11, retorno.*());
			stmtInsertLista.setString(12, retorno.*());
			stmtInsertLista.executeUpdate();
		} catch (SQLException e) {
			retorno.toString();
			throw new SQLException(e);				
		}
	}

}

}[/code]

O meu colega opta por algo como tudo junto na mesma função . o que vocês acham que seria o melhor

nesse caso eu usaria tudo na mesma funcao, ja que as duas fazem parte do mesmo contexto.

sim mas concorda que essa tripa de 50 linhas eh pior do que elas modularizadas ?

Mas tamanho de método não tem nada haver com o principio de acoplamento ou coesão.

Pode não ter a ver com coesão e acoplamento porém é uma boa prática que melhora a leitura e o entendimento do código. O ideal é que uma função tenha no máximo 20 linhas e que cada função tenha apenas um papel.

na verdade não concordo.

impossível fazer uma função com no máximo 20 linhas, ainda mais se for uma função de insert usando JDBC.

Nada a ver esse lance de montar uma função com apenas 20 linhas, dependendo da situação e de qual a finalidade da mesma fazer em apenas 20 linhas e loucura, se tornaria algo ilegível.

[quote=romarcio]Mas tamanho de método não tem nada haver com o principio de acoplamento ou coesão.
[/quote]

Sim o tamanho do método não tem nada a ver com acoplamento ou coesão .
O argumento de meu colega é que ao quebrar essa função(que pode ser transformada em uma função de 50 linhas) o meu código tem alto nível de acoplamento . Porque tem uma função chamando a outra e isso não é uma boa pratica de programação.

O meu argumento é que ao modularizar essa função deixo o código mais limpo fácil de entender fácil de testar e fácil de reaproveitar ( as vezes enchergar o reaproveitamento).

Não vejo nada relacionado a acoplamento ai, uma vez que os metodos em questão são todos da classe, de forma que não haveria nenhuma interferência exterior nisso. Porém eu não faria isso, antes de dividir alguma parte do código analiso primeiro se este será reutilizado am algum outro metodo publico da classe e ai sim irei criar um metodo privado a parte, dependendo do metodo acho que ficaria inviavel dividilo em varias parte que não seriam reutilizadas e que deixariam tudo fatiado fazendo com que para depurar um metodo ou acompanhalo visualmente tivesse que ficar navegando emtre varios outros.

Então leandrow3b e mauricioadl eu penso diferente.

Uma vez escutei isso de um professor e concordei com ele.Toda vez que você ta repetindo (como no meu caso mesmo) tem alguma coisa errada.
Repetir é coisa pra computador fazer.
Então eu poderia fazer um laço para os insert e não repetir codigo até 13.

[code]for (Field f : retorno.getClass().getFields()){
f.setAccessible(true);
f.get(retorno);
stmtInsertLista.setString(i, (String) f.get(retorno));

			}
			[/code]

Também não tem nada haver com acoplamento. Uma classe tem alto acoplamento com outra classe e não com ela mesmo.

[quote]Acoplamento é o grau em que uma classe conhece outra classe. Se o único conhecimento que a classe A tem sobre a classe B é o que a classe B expôs através de sua interface, então diz-se das classe A e B que elas possuem um baixo acoplamento… E isso é bom.
Se, por outro lado, a classe A depende de partes da classe B que não fazem parte da interface da classe B, então o acoplamento entre as classes é mais alto. O que não é bom.
Entre outras palavras, se A sabe mais do que deveria sobre a maneira pela qual B foi implementada, então A e B tem alto acoplamento.
[/quote]

Para o amigo q gosta de spaguetti code recomendo um livro chamado Clean Code. Ao contrárioo do q disse, qnto menor o número de linhas em um método, mais legível ele fica.

Acoplamento e coesão são princípios que devem ser podem e devem ser observado em métodos também, mesmo eles pertencendo a mesma classe.

Mesmo sendo um smell, não podemos decidir se um método deve ser dividio em dois ou mais apenas pelo tamanho, isso é apenas um indicativo, e não um critério. Na minha opinião, a principal observação a ser feita é se o método é coeso, ou seja, se ele faz apenas 1 única tarefa e se essa tarefa não está espalhada em diversos outros métodos, ou seja, o ideal é que a relação entre método <=> tarefa seja 1:1.

Na minha opinião, esse código ficaria melhor em um único método sim. Observe que os métodos private nesse caso não tem sentido fora do contexto do método exportarDadosRetorno, isso é um indicativo de que esse código deveria fazer parte de um único método.

Na pior das hipóteses, faça o teste e compare :

public class Transfer {
	public static void exportarDadosRetorno(ListaRetorno listaRetorno) throws TransferException, ConexaoException {
		Connection con = null;
		
		try (con = ConexaoCop.iniciarConexao()) {	
			PreparedStatement stmtInsertLista = con.prepareStatement("INSERT INTO tabela*(*, *, *, *, *, *, *,*, *, *, *, *) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)");

			for (Retorno retorno : listaRetorno.getLista()) {
				stmtInsertLista.setString(1, retorno.*());
				stmtInsertLista.setTimestamp(2, retorno.*());
				stmtInsertLista.setString(3, retorno.*());
				stmtInsertLista.setString(4, retorno.*());
				stmtInsertLista.setString(5, retorno.*());
				stmtInsertLista.setString(6, retorno.*());
				stmtInsertLista.setString(7, retorno.*());
				stmtInsertLista.setString(8, retorno.*());
				stmtInsertLista.setString(9, retorno.*());
				stmtInsertLista.setString(10, retorno.*());
				stmtInsertLista.setString(11, retorno.*());
				stmtInsertLista.setString(12, retorno.*());
				stmtInsertLista.executeUpdate();
			}
			
			con.commit();
		} catch (SQLException e) {
			if(con != null){
				con.rollback();
			}
			
			throw new TransferException("Erro ao acessar banco de dados para exportar Dados: " + retorno, e);
		}
	}
}

Po galera brigado pelas sujestões e ajuda a versão final fico assim:

[code] public static void exportarDadosRetorno(ListaRetorno listaRetorno)
throws TransferException{

	try(Connection con = Conexao.iniciarConexao()){			
		PreparedStatement stmtInsertLista = con.prepareStatement("INSERT INTO tabela(*, *, *, *, *, *, *,"
				+ "*, *, *, *, *) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)");
		for (Retorno retorno : listaRetorno.getLista()) {
			stmtInsertLista.setTimestamp(1, retorno.getDataProcessamento());
			Field[] campo = retorno.getClass().getDeclaredFields();			
			
			for(int i = 2; i < campo.length ; i++){
				campo[i].setAccessible(true);					
					stmtInsertLista.setString(i, (String) campo[i].get(retorno));			
			}
			stmtInsertLista.executeUpdate();
		}
		con.commit();
	} 
	catch (SQLException e) {				
		throw new TransferException("Erro ao acessar banco de dados para exportar Dados de Retorno", e);
	}catch (IllegalArgumentException | IllegalAccessException e) {
		LOG.error("Erro Transferindo Registro de retorno: ", e);
		throw new TransferException("Erro ao acessar banco de dados para exportar Dados de Retorno", e);
	} catch (ConexaoException e) {
		LOG.error("Erro ao iniciar conexão para transferir registros de retorno", e);
		throw new TransferException("Erro ao iniciar conexão para transferir registros de retorno", e);
	} 	
}[/code]

Dizer a verdade meu gestor me orientou no uso do Reflection. Ela so pode ser usada na construção de libs e afims porque no ultimo código eu estaria violando brutalmente o encapsulamento.
Então a versão final ficou assim.

[code]public static void exportarDadosRetorno(ListaRetorno listaRetorno)
throws TransferException{

	try(Connection con = ConexaoCop.iniciarConexao()){			
		PreparedStatement stmtInsertLista = con.prepareStatement("INSERT INTO tabela(*, *, *, *, *, *, *,"
				+ "*, *, *, *, *) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)");

		for (Retorno retorno : listaRetorno.getLista()) {
			stmtInsertLista.setString(1, retorno.*());
			stmtInsertLista.setTimestamp(2, retorno.*());
			stmtInsertLista.setString(3, retorno.*());
			stmtInsertLista.setString(4, retorno.*());
			stmtInsertLista.setString(5, retorno.*());
			stmtInsertLista.setString(6, retorno.*());
			stmtInsertLista.setString(7, retorno.*());
			stmtInsertLista.setString(8, retorno.*());
			stmtInsertLista.setString(9, retorno.*());
			stmtInsertLista.setString(10, retorno.*());
			stmtInsertLista.setString(11, retorno.*());
			stmtInsertLista.setString(12, retorno.*());
			stmtInsertLista.executeUpdate();
		}
		con.commit();
	} 
	catch (SQLException e) {				
		throw new TransferException("Erro ao acessar banco de dados para exportar Dados de Retorno", e);
	} catch (ConexaoException e) {
		LOG.error("Erro ao iniciar conexão para transferir registros de retorno", e);
		throw new TransferException("Erro ao iniciar conexão para transferir registros de retorno", e);
	} 	
}[/code]