Eu tenho lido Trabalhando Efetivamente com o Código Legado e o Código Limpo, com o objetivo de aprender estratégias sobre como começar a limpar a base de código existente de um aplicativo de formulários da Web ASP.NET grande.
Este sistema existe desde 2005 e desde então passou por várias melhorias. Originalmente, o código foi estruturado da seguinte maneira (e ainda é amplamente estruturado dessa maneira):
- ASP.NET (aspx / ascx)
- Code-behind (c #)
- Camada lógica de negócios (c #)
- Camada de acesso a dados (c #)
- Banco de Dados (Oracle)
A questão principal é que o código é disfarçado de procedimento como orientado a objetos. Ele praticamente viola todas as diretrizes descritas nos dois livros.
Este é um exemplo de uma classe típica na camada lógica de negócios:
public class AddressBO
{
public TransferObject GetAddress(string addressID)
{
if (StringUtils.IsNull(addressID))
{
throw new ValidationException("Address ID must be entered");
}
AddressDAO addressDAO = new AddressDAO();
return addressDAO.GetAddress(addressID);
}
public TransferObject Insert(TransferObject addressDetails)
{
if (StringUtils.IsNull(addressDetails.GetString("EVENT_ID")) ||
StringUtils.IsNull(addressDetails.GetString("LOCALITY")) ||
StringUtils.IsNull(addressDetails.GetString("ADDRESS_TARGET")) ||
StringUtils.IsNull(addressDetails.GetString("ADDRESS_TYPE_CODE")) ||
StringUtils.IsNull(addressDetails.GetString("CREATED_BY")))
{
throw new ValidationException(
"You must enter an Event ID, Locality, Address Target, Address Type Code and Created By.");
}
string addressID = Sequence.GetNextValue("ADDRESS_ID_SEQ");
addressDetails.SetValue("ADDRESS_ID", addressID);
string syncID = Sequence.GetNextValue("SYNC_ID_SEQ");
addressDetails.SetValue("SYNC_ADDRESS_ID", syncID);
TransferObject syncDetails = new TransferObject();
Transaction transaction = new Transaction();
try
{
AddressDAO addressDAO = new AddressDAO();
addressDAO.Insert(addressDetails, transaction);
// insert the record for the target
TransferObject addressTargetDetails = new TransferObject();
switch (addressDetails.GetString("ADDRESS_TARGET"))
{
case "PARTY_ADDRESSES":
{
addressTargetDetails.SetValue("ADDRESS_ID", addressID);
addressTargetDetails.SetValue("ADDRESS_TYPE_CODE",
addressDetails.GetString("ADDRESS_TYPE_CODE"));
addressTargetDetails.SetValue("PARTY_ID", addressDetails.GetString("PARTY_ID"));
addressTargetDetails.SetValue("EVENT_ID", addressDetails.GetString("EVENT_ID"));
addressTargetDetails.SetValue("CREATED_BY", addressDetails.GetString("CREATED_BY"));
addressDAO.InsertPartyAddress(addressTargetDetails, transaction);
break;
}
case "PARTY_CONTACT_ADDRESSES":
{
addressTargetDetails.SetValue("ADDRESS_ID", addressID);
addressTargetDetails.SetValue("ADDRESS_TYPE_CODE",
addressDetails.GetString("ADDRESS_TYPE_CODE"));
addressTargetDetails.SetValue("PUBLIC_RELEASE_FLAG",
addressDetails.GetString("PUBLIC_RELEASE_FLAG"));
addressTargetDetails.SetValue("CONTACT_ID", addressDetails.GetString("CONTACT_ID"));
addressTargetDetails.SetValue("EVENT_ID", addressDetails.GetString("EVENT_ID"));
addressTargetDetails.SetValue("CREATED_BY", addressDetails.GetString("CREATED_BY"));
addressDAO.InsertContactAddress(addressTargetDetails, transaction);
break;
}
<< many more cases here >>
default:
{
break;
}
}
// synchronise
SynchronisationBO synchronisationBO = new SynchronisationBO();
syncDetails = synchronisationBO.Synchronise("I", transaction,
"ADDRESSES", addressDetails.GetString("ADDRESS_TARGET"),
addressDetails, addressTargetDetails);
// commit
transaction.Commit();
}
catch (Exception)
{
transaction.Rollback();
throw;
}
return new TransferObject("ADDRESS_ID", addressID, "SYNC_DETAILS", syncDetails);
}
<< many more methods are here >>
}
Tem muita duplicação, a classe tem várias responsabilidades, etc, etc - geralmente é um código 'não limpo'.
Todo o código em todo o sistema depende de implementações concretas.
Este é um exemplo de uma classe típica na camada de acesso a dados:
public class AddressDAO : GenericDAO
{
public static readonly string BASE_SQL_ADDRESSES =
"SELECT " +
" a.address_id, " +
" a.event_id, " +
" a.flat_unit_type_code, " +
" fut.description as flat_unit_description, " +
" a.flat_unit_num, " +
" a.floor_level_code, " +
" fl.description as floor_level_description, " +
" a.floor_level_num, " +
" a.building_name, " +
" a.lot_number, " +
" a.street_number, " +
" a.street_name, " +
" a.street_type_code, " +
" st.description as street_type_description, " +
" a.street_suffix_code, " +
" ss.description as street_suffix_description, " +
" a.postal_delivery_type_code, " +
" pdt.description as postal_delivery_description, " +
" a.postal_delivery_num, " +
" a.locality, " +
" a.state_code, " +
" s.description as state_description, " +
" a.postcode, " +
" a.country, " +
" a.lock_num, " +
" a.created_by, " +
" TO_CHAR(a.created_datetime, '" + SQL_DATETIME_FORMAT + "') as created_datetime, " +
" a.last_updated_by, " +
" TO_CHAR(a.last_updated_datetime, '" + SQL_DATETIME_FORMAT + "') as last_updated_datetime, " +
" a.sync_address_id, " +
" a.lat," +
" a.lon, " +
" a.validation_confidence, " +
" a.validation_quality, " +
" a.validation_status " +
"FROM ADDRESSES a, FLAT_UNIT_TYPES fut, FLOOR_LEVELS fl, STREET_TYPES st, " +
" STREET_SUFFIXES ss, POSTAL_DELIVERY_TYPES pdt, STATES s " +
"WHERE a.flat_unit_type_code = fut.flat_unit_type_code(+) " +
"AND a.floor_level_code = fl.floor_level_code(+) " +
"AND a.street_type_code = st.street_type_code(+) " +
"AND a.street_suffix_code = ss.street_suffix_code(+) " +
"AND a.postal_delivery_type_code = pdt.postal_delivery_type_code(+) " +
"AND a.state_code = s.state_code(+) ";
public TransferObject GetAddress(string addressID)
{
//Build the SELECT Statement
StringBuilder selectStatement = new StringBuilder(BASE_SQL_ADDRESSES);
//Add WHERE condition
selectStatement.Append(" AND a.address_id = :addressID");
ArrayList parameters = new ArrayList{DBUtils.CreateOracleParameter("addressID", OracleDbType.Decimal, addressID)};
// Execute the SELECT statement
Query query = new Query();
DataSet results = query.Execute(selectStatement.ToString(), parameters);
// Check if 0 or more than one rows returned
if (results.Tables[0].Rows.Count == 0)
{
throw new NoDataFoundException();
}
if (results.Tables[0].Rows.Count > 1)
{
throw new TooManyRowsException();
}
// Return a TransferObject containing the values
return new TransferObject(results);
}
public void Insert(TransferObject insertValues, Transaction transaction)
{
// Store Values
string addressID = insertValues.GetString("ADDRESS_ID");
string syncAddressID = insertValues.GetString("SYNC_ADDRESS_ID");
string eventID = insertValues.GetString("EVENT_ID");
string createdBy = insertValues.GetString("CREATED_BY");
// postal delivery
string postalDeliveryTypeCode = insertValues.GetString("POSTAL_DELIVERY_TYPE_CODE");
string postalDeliveryNum = insertValues.GetString("POSTAL_DELIVERY_NUM");
// unit/building
string flatUnitTypeCode = insertValues.GetString("FLAT_UNIT_TYPE_CODE");
string flatUnitNum = insertValues.GetString("FLAT_UNIT_NUM");
string floorLevelCode = insertValues.GetString("FLOOR_LEVEL_CODE");
string floorLevelNum = insertValues.GetString("FLOOR_LEVEL_NUM");
string buildingName = insertValues.GetString("BUILDING_NAME");
// street
string lotNumber = insertValues.GetString("LOT_NUMBER");
string streetNumber = insertValues.GetString("STREET_NUMBER");
string streetName = insertValues.GetString("STREET_NAME");
string streetTypeCode = insertValues.GetString("STREET_TYPE_CODE");
string streetSuffixCode = insertValues.GetString("STREET_SUFFIX_CODE");
// locality/state/postcode/country
string locality = insertValues.GetString("LOCALITY");
string stateCode = insertValues.GetString("STATE_CODE");
string postcode = insertValues.GetString("POSTCODE");
string country = insertValues.GetString("COUNTRY");
// esms address
string esmsAddress = insertValues.GetString("ESMS_ADDRESS");
//address/GPS
string lat = insertValues.GetString("LAT");
string lon = insertValues.GetString("LON");
string zoom = insertValues.GetString("ZOOM");
//string validateDate = insertValues.GetString("VALIDATED_DATE");
string validatedBy = insertValues.GetString("VALIDATED_BY");
string confidence = insertValues.GetString("VALIDATION_CONFIDENCE");
string status = insertValues.GetString("VALIDATION_STATUS");
string quality = insertValues.GetString("VALIDATION_QUALITY");
// the insert statement
StringBuilder insertStatement = new StringBuilder("INSERT INTO ADDRESSES (");
StringBuilder valuesStatement = new StringBuilder("VALUES (");
ArrayList parameters = new ArrayList();
// build the insert statement
insertStatement.Append("ADDRESS_ID, EVENT_ID, CREATED_BY, CREATED_DATETIME, LOCK_NUM ");
valuesStatement.Append(":addressID, :eventID, :createdBy, SYSDATE, 1 ");
parameters.Add(DBUtils.CreateOracleParameter("addressID", OracleDbType.Decimal, addressID));
parameters.Add(DBUtils.CreateOracleParameter("eventID", OracleDbType.Decimal, eventID));
parameters.Add(DBUtils.CreateOracleParameter("createdBy", OracleDbType.Varchar2, createdBy));
// build the insert statement
if (!StringUtils.IsNull(syncAddressID))
{
insertStatement.Append(", SYNC_ADDRESS_ID");
valuesStatement.Append(", :syncAddressID");
parameters.Add(DBUtils.CreateOracleParameter("syncAddressID", OracleDbType.Decimal, syncAddressID));
}
if (!StringUtils.IsNull(postalDeliveryTypeCode))
{
insertStatement.Append(", POSTAL_DELIVERY_TYPE_CODE");
valuesStatement.Append(", :postalDeliveryTypeCode ");
parameters.Add(DBUtils.CreateOracleParameter("postalDeliveryTypeCode", OracleDbType.Varchar2, postalDeliveryTypeCode));
}
if (!StringUtils.IsNull(postalDeliveryNum))
{
insertStatement.Append(", POSTAL_DELIVERY_NUM");
valuesStatement.Append(", :postalDeliveryNum ");
parameters.Add(DBUtils.CreateOracleParameter("postalDeliveryNum", OracleDbType.Varchar2, postalDeliveryNum));
}
if (!StringUtils.IsNull(flatUnitTypeCode))
{
insertStatement.Append(", FLAT_UNIT_TYPE_CODE");
valuesStatement.Append(", :flatUnitTypeCode ");
parameters.Add(DBUtils.CreateOracleParameter("flatUnitTypeCode", OracleDbType.Varchar2, flatUnitTypeCode));
}
if (!StringUtils.IsNull(lat))
{
insertStatement.Append(", LAT");
valuesStatement.Append(", :lat ");
parameters.Add(DBUtils.CreateOracleParameter("lat", OracleDbType.Decimal, lat));
}
if (!StringUtils.IsNull(lon))
{
insertStatement.Append(", LON");
valuesStatement.Append(", :lon ");
parameters.Add(DBUtils.CreateOracleParameter("lon", OracleDbType.Decimal, lon));
}
if (!StringUtils.IsNull(zoom))
{
insertStatement.Append(", ZOOM");
valuesStatement.Append(", :zoom ");
parameters.Add(DBUtils.CreateOracleParameter("zoom", OracleDbType.Decimal, zoom));
}
if (!StringUtils.IsNull(flatUnitNum))
{
insertStatement.Append(", FLAT_UNIT_NUM");
valuesStatement.Append(", :flatUnitNum ");
parameters.Add(DBUtils.CreateOracleParameter("flatUnitNum", OracleDbType.Varchar2, flatUnitNum));
}
if (!StringUtils.IsNull(floorLevelCode))
{
insertStatement.Append(", FLOOR_LEVEL_CODE");
valuesStatement.Append(", :floorLevelCode ");
parameters.Add(DBUtils.CreateOracleParameter("floorLevelCode", OracleDbType.Varchar2, floorLevelCode));
}
if (!StringUtils.IsNull(floorLevelNum))
{
insertStatement.Append(", FLOOR_LEVEL_NUM");
valuesStatement.Append(", :floorLevelNum ");
parameters.Add(DBUtils.CreateOracleParameter("floorLevelNum", OracleDbType.Varchar2, floorLevelNum));
}
if (!StringUtils.IsNull(buildingName))
{
insertStatement.Append(", BUILDING_NAME");
valuesStatement.Append(", :buildingName ");
parameters.Add(DBUtils.CreateOracleParameter("buildingName", OracleDbType.Varchar2, buildingName));
}
if (!StringUtils.IsNull(lotNumber))
{
insertStatement.Append(", LOT_NUMBER");
valuesStatement.Append(", :lotNumber ");
parameters.Add(DBUtils.CreateOracleParameter("lotNumber", OracleDbType.Varchar2, lotNumber));
}
if (!StringUtils.IsNull(streetNumber))
{
insertStatement.Append(", STREET_NUMBER");
valuesStatement.Append(", :streetNumber ");
parameters.Add(DBUtils.CreateOracleParameter("streetNumber", OracleDbType.Varchar2, streetNumber));
}
if (!StringUtils.IsNull(streetName))
{
insertStatement.Append(", STREET_NAME");
valuesStatement.Append(", :streetName ");
parameters.Add(DBUtils.CreateOracleParameter("streetName", OracleDbType.Varchar2, streetName));
}
if (!StringUtils.IsNull(streetTypeCode))
{
insertStatement.Append(", STREET_TYPE_CODE");
valuesStatement.Append(", :streetTypeCode ");
parameters.Add(DBUtils.CreateOracleParameter("streetTypeCode", OracleDbType.Varchar2, streetTypeCode));
}
if (!StringUtils.IsNull(streetSuffixCode))
{
insertStatement.Append(", STREET_SUFFIX_CODE");
valuesStatement.Append(", :streetSuffixCode ");
parameters.Add(DBUtils.CreateOracleParameter("streetSuffixCode", OracleDbType.Varchar2, streetSuffixCode));
}
if (!StringUtils.IsNull(locality))
{
insertStatement.Append(", LOCALITY");
valuesStatement.Append(", :locality");
parameters.Add(DBUtils.CreateOracleParameter("locality", OracleDbType.Varchar2, locality));
}
if (!StringUtils.IsNull(stateCode))
{
insertStatement.Append(", STATE_CODE");
valuesStatement.Append(", :stateCode");
parameters.Add(DBUtils.CreateOracleParameter("stateCode", OracleDbType.Varchar2, stateCode));
}
if (!StringUtils.IsNull(postcode))
{
insertStatement.Append(", POSTCODE");
valuesStatement.Append(", :postcode ");
parameters.Add(DBUtils.CreateOracleParameter("postcode", OracleDbType.Varchar2, postcode));
}
if (!StringUtils.IsNull(country))
{
insertStatement.Append(", COUNTRY");
valuesStatement.Append(", :country ");
parameters.Add(DBUtils.CreateOracleParameter("country", OracleDbType.Varchar2, country));
}
if (!StringUtils.IsNull(esmsAddress))
{
insertStatement.Append(", ESMS_ADDRESS");
valuesStatement.Append(", :esmsAddress ");
parameters.Add(DBUtils.CreateOracleParameter("esmsAddress", OracleDbType.Varchar2, esmsAddress));
}
if (!StringUtils.IsNull(validatedBy))
{
insertStatement.Append(", VALIDATED_DATE");
valuesStatement.Append(", SYSDATE ");
insertStatement.Append(", VALIDATED_BY");
valuesStatement.Append(", :validatedBy ");
parameters.Add(DBUtils.CreateOracleParameter("validatedBy", OracleDbType.Varchar2, validatedBy));
}
if (!StringUtils.IsNull(confidence))
{
insertStatement.Append(", VALIDATION_CONFIDENCE");
valuesStatement.Append(", :confidence ");
parameters.Add(DBUtils.CreateOracleParameter("confidence", OracleDbType.Decimal, confidence));
}
if (!StringUtils.IsNull(status))
{
insertStatement.Append(", VALIDATION_STATUS");
valuesStatement.Append(", :status ");
parameters.Add(DBUtils.CreateOracleParameter("status", OracleDbType.Varchar2, status));
}
if (!StringUtils.IsNull(quality))
{
insertStatement.Append(", VALIDATION_QUALITY");
valuesStatement.Append(", :quality ");
parameters.Add(DBUtils.CreateOracleParameter("quality", OracleDbType.Decimal, quality));
}
// finish off the statement
insertStatement.Append(") ");
valuesStatement.Append(")");
// build the insert statement
string sql = insertStatement + valuesStatement.ToString();
// Execute the INSERT Statement
Dml dmlDAO = new Dml();
int rowsAffected = dmlDAO.Execute(sql, transaction, parameters);
if (rowsAffected == 0)
{
throw new NoRowsAffectedException();
}
}
<< many more methods go here >>
}
Este sistema foi desenvolvido por mim e uma pequena equipe em 2005, após um curso .NET de 1 semana. Antes do que minha experiência era em aplicativos cliente-servidor. Nos últimos 5 anos, reconheci os benefícios dos testes de unidade automatizados, testes de integração automatizados e testes de aceitação automatizados (usando Selenium ou equivalente), mas a base de código atual parece impossível de introduzir esses conceitos.
Agora estamos começando a trabalhar em um grande projeto de aprimoramento com prazos apertados. A equipe é composta por 5 desenvolvedores .NET - 2 desenvolvedores com alguns anos de experiência em .NET e 3 outros com pouca ou nenhuma experiência em .NET. Nenhum membro da equipe (inclusive eu) tem experiência no uso de estruturas de teste ou simulação de unidades .NET.
Que estratégia você usaria para tornar esse código mais limpo, mais orientado a objetos, testável e sustentável?
c#
.net
unit-testing
object-oriented
Anthony
fonte
fonte
Respostas:
Você menciona dois livros nos quais uma das principais mensagens é "A regra dos escoteiros", ou seja, limpe o código conforme você o toca. Se você possui um sistema operacional, uma reescrita maciça é contraproducente. Em vez disso, à medida que você adiciona novas funcionalidades, melhore o código como está.
Para aprofundar, Feathers fala sobre testar a aplicação em suas costuras: os pontos lógicos nos quais as unidades se conectam. Você pode tirar proveito de uma costura para criar um esboço ou simulação para uma dependência, para poder escrever testes em torno do objeto dependente. Vamos dar o seu AddressBO como um exemplo
Existe uma costura óbvia entre o AddressBO e o AddressDAO. Vamos criar uma interface para o AddressDAO e permitir que a dependência seja injetada no AddressBO.
Agora você prepara seu AddressBO para permitir a injeção
Aqui estamos usando "injeção de dependência do pobre homem". Nosso único objetivo é quebrar a costura e nos permitir testar o AddressBO. Agora, em nossos testes de unidade, podemos criar um IAddressDAO simulado e validar as interações entre os dois objetos.
fonte
Se bem me lembro, Trabalhando Efetivamente com o Código Legado diz que uma reescrita completa não garante que o novo código seja melhor que o antigo (do ponto de vista da funcionalidade / defeitos). As refatorações nesse livro são para corrigir bugs / adicionar novos recursos.
Outro livro que eu recomendaria é o Brownfield Application Development no .NET, que basicamente diz para não fazer uma reescrita completa também. Ele fala sobre fazer mudanças constantes e iterativas sempre que você adiciona novos recursos ou corrige defeitos. Ele aborda as considerações de custo x benefícios e alerta sobre a tentativa de diminuir demais ao mesmo tempo. Enquanto o Working Effective with Legacy Code fala principalmente sobre como refatorar no nível micro / código, o Brownfield Application Development no .NET cobre principalmente as considerações de nível mais alto ao refatorar (junto com algumas coisas no nível do código).
O livro Brownfield também sugere descobrir qual área do código está causando mais problemas e focar nela. Qualquer outra área que não precise de muita manutenção pode não valer a pena mudar.
fonte
Para um aplicativo herdado, é muito mais econômico começar cobrindo-o com testes de integração de nível superior (automatizados) em vez de testes de unidade. Depois, com os testes de integração como sua rede de segurança, você pode começar a refatorar em pequenas etapas, se apropriado, ou seja, se o custo da refatoração se pagar a longo prazo. Como outros observaram, isso não é evidente.
Veja também esta minha resposta anterior a uma pergunta semelhante; espero que você ache útil.
fonte
Tenha muito cuidado ao jogar fora e reescrever o código em execução ( coisas que você nunca deve fazer ). Claro que pode ser feio, mas se funcionar, deixe estar. Veja a publicação no blog de Joel, com certeza com mais de 10 anos, mas ainda está no alvo.
fonte
Como Mike afirmou que a 'regra dos escoteiros' é provavelmente a melhor aqui, se o código funcionar e não for uma fonte constante de relatórios de erros, eu preferiria deixá-lo lá e melhorá-lo lentamente ao longo do tempo.
Durante o seu projeto de aprimoramento, permita novas maneiras de fazer as coisas. Por exemplo, use um ORM para novos recursos e ignore o padrão da camada de dados existente. Ao encontrar aprimoramentos que precisam tocar no código existente, você pode mover parte do código relacionado para a nova maneira de fazer as coisas. Usar uma fachada ou alguns adaptadores em locais pode ajudar a isolar o código antigo, talvez até por camada. Isso pode ajudá-lo a passar fome com o código antigo com o tempo.
Da mesma forma, isso pode ajudá-lo a adicionar testes de unidade, você pode começar com o novo código que você faz e adicionar lentamente alguns testes ao código antigo que você precisa tocar para obter os novos aprimoramentos.
fonte
Esses são os dois bons livros. Se você começar a reescrever o código dessa maneira, acho importante também começar a cobrir o código com testes de unidade para ajudar a mantê-lo estável enquanto você o reescreve.
Isso deve ser feito em pequenas etapas e a modificação desse tipo de código pode facilmente desestabilizar todo o sistema.
Eu não modificaria nenhum código no qual você não esteja trabalhando ativamente. Faça isso apenas no código que você está aprimorando ou corrigindo ativamente. Se algo está servindo a seu propósito, mas não foi modificado há anos, basta deixá-lo. Está fazendo, mesmo se você souber uma maneira melhor.
No final do dia, a empresa precisa de produtividade. Embora um código melhor aumente o código de reescrita da produtividade, apenas porque ele poderia ser melhor escrito provavelmente não é a melhor maneira de agregar valor ao seu produto.
fonte