Czytałem Efektywna praca ze starszymi kodami i czystym kodem w celu opracowania strategii uczenia się, w jaki sposób rozpocząć czyszczenie istniejącej bazy kodu dużej aplikacji WWW ASP.NET.
System ten istnieje od 2005 r. I od tego czasu został poddany wielu udoskonaleniom. Pierwotnie kod miał następującą strukturę (i nadal jest w dużej mierze zbudowany w ten sposób):
- ASP.NET (aspx / ascx)
- Kod z tyłu (c #)
- Warstwa logiki biznesowej (c #)
- Warstwa dostępu do danych (c #)
- Baza danych (Oracle)
Głównym problemem jest to, że kod podszywa się pod proceduralnie jako obiektowy. To praktycznie narusza wszystkie wytyczne opisane w obu książkach.
To jest przykład typowej klasy w warstwie logiki biznesowej:
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 >>
}Ma wiele powielania, klasa ma szereg obowiązków itp. Itd. - jest to po prostu kod „nieczyszczony”.
Cały kod w całym systemie zależy od konkretnych implementacji.
To jest przykład typowej klasy w warstwie dostępu do danych:
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 >>
}System ten został opracowany przeze mnie i mały zespół w 2005 roku po tygodniowym kursie .NET. Wcześniej moje doświadczenie dotyczyło aplikacji klient-serwer. W ciągu ostatnich 5 lat zauważyłem zalety automatycznych testów jednostkowych, automatycznych testów integracji i automatycznych testów akceptacyjnych (przy użyciu Selenium lub równoważnego), ale obecna baza kodu wydaje się niemożliwa do wprowadzenia tych koncepcji.
Zaczynamy teraz pracować nad dużym projektem udoskonalenia z napiętymi ramami czasowymi. Zespół składa się z 5 programistów .NET - 2 programistów z kilkuletnim doświadczeniem .NET i 3 innych z niewielkim lub zerowym doświadczeniem .NET. Żaden z członków zespołu (w tym ja) nie ma doświadczenia w używaniu testowania jednostkowego .NET lub kpowania z frameworków.
Jakiej strategii użyłbyś, aby uczynić ten kod czystszym, bardziej obiektowym, testowalnym i łatwym w utrzymaniu?
c#
.net
unit-testing
object-oriented
Anthony
źródło
źródło
Odpowiedzi:
Wspominasz o dwóch książkach, w których jedną z głównych wiadomości jest „Reguła skautowa”, tj. Wyczyść kod po jego dotknięciu. Jeśli masz działający system, masowe przepisywanie przyniesie efekt przeciwny do zamierzonego. Zamiast tego, dodając nową funkcjonalność, popraw kod w jego obecnej postaci.
Aby zagłębić się dalej, Feathers mówi o testowaniu aplikacji w jej szwach: logiczne punkty, w których łączą się jednostki. Możesz skorzystać ze szwu, aby utworzyć kod pośredniczący lub próbny dla zależności, aby móc pisać testy wokół obiektu zależnego. Weźmy twój AddressBO jako przykład
Istnieje oczywisty szew między AddressBO a AddressDAO. Utwórzmy interfejs dla AddressDAO i pozwólmy na wstrzyknięcie zależności do AddressBO.
Teraz dokonasz dokowania adresu AddressBO, aby umożliwić wstrzyknięcie
Tutaj używamy „zastrzyku zależności biednego człowieka”. Naszym jedynym celem jest przełamanie szwu i umożliwienie nam przetestowania AddressBO. Teraz w naszych testach jednostkowych możemy wykonać próbny IAddressDAO i zweryfikować interakcje między dwoma obiektami.
źródło
Jeśli dobrze pamiętam, Efektywna praca ze starszym kodem mówi, że pełne przepisanie nie gwarantuje, że nowy kod będzie lepszy od starego (z punktu widzenia funkcjonalności / wad). Refaktoryzacje w tej książce służą do naprawiania błędów / dodawania nowych funkcji.
Kolejną książką, którą poleciłbym, jest Brownfield Application Development w .NET, która w zasadzie mówi, aby nie robić także pełnego przepisywania. Mówi o wprowadzaniu stałych, iteracyjnych zmian za każdym razem, gdy dodajesz nowe funkcje lub naprawiasz wady. Odnosi się do rozważań dotyczących kosztów i korzyści i ostrzega przed próbą zbytniego odgryzienia za jednym razem. Podczas gdy Efektywna praca ze starszym kodem mówi głównie o tym, jak refaktoryzować na poziomie mikro / kodu, Brownfield Application Development w .NET , obejmuje głównie kwestie wyższego poziomu przy ponownym faktorowaniu (wraz z niektórymi rzeczami na poziomie kodu).
Książka Brownfielda sugeruje także, aby dowiedzieć się, który obszar kodu powoduje najwięcej problemów i skupić się tam. Wszelkie inne obszary, które nie wymagają dużej konserwacji, mogą nie być warte zmiany.
źródło
W przypadku takiej starszej aplikacji znacznie bardziej opłacalne jest rozpoczęcie od objęcia jej (automatycznymi) testami integracji wyższego poziomu, a nie testami jednostkowymi. Następnie z testami integracyjnymi jako siatką bezpieczeństwa, możesz rozpocząć refaktoryzację małymi krokami, jeśli jest to właściwe, tj. Jeśli koszt refaktoryzacji zwróci się w dłuższej perspektywie. Jak zauważyli inni, nie jest to oczywiste.
Zobacz także moją wcześniejszą odpowiedź na podobne pytanie; mam nadzieję, że okażą się przydatne.
źródło
Bądź bardzo ostrożny przy wyrzucaniu i przepisywaniu działającego kodu ( rzeczy, których nigdy nie powinieneś robić ). Jasne, może to być brzydkie, ale jeśli działa, zostaw to. Zobacz post na blogu Joela, z pewnością ma ponad 10 lat, ale nadal jest trafiony w cel.
źródło
Jak stwierdził Mike, „zasada zwiadowcy” jest prawdopodobnie najlepsza tutaj, jeśli kod działa i nie jest stałym źródłem raportów o błędach, wolałbym pozwolić mu tam siedzieć i powoli go ulepszać.
Podczas projektu ulepszeń uwzględnij nowe sposoby robienia rzeczy. Na przykład użyj ORM dla nowych funkcji i pomiń istniejący wzorzec warstwy danych. Po napotkaniu ulepszeń, które muszą dotykać istniejącego kodu, możesz być w stanie przenieść część pokrewnego kodu na nowy sposób robienia rzeczy. Użycie fasady lub niektórych adapterów w niektórych miejscach może pomóc w wyodrębnieniu starego kodu, być może nawet na warstwę. To może pomóc ci zagłodzić stary kod z czasem.
Podobnie może to pomóc w dodawaniu testów jednostkowych, możesz zacząć od nowego kodu, który tworzysz i powoli dodawać testy dla starego kodu, którego musisz dotknąć, aby uzyskać nowe ulepszenia.
źródło
To są dobre książki. Jeśli zamierzasz zacząć przepisywać kod w ten sposób, myślę, że ważne jest, aby zacząć również obejmować kod testami jednostkowymi, aby pomóc zachować stabilność podczas przepisywania.
Należy to zrobić małymi krokami, a modyfikacja tego rodzaju kodu może łatwo zdestabilizować cały system.
Nie zmodyfikowałbym żadnego kodu, nad którym nie aktywnie pracujesz. Rób to tylko na kodzie, który aktywnie poprawiasz lub naprawiasz. Jeśli coś służy swojemu celowi, ale nie było modyfikowane od lat, po prostu to zostaw. Robi to, nawet jeśli znasz lepszy sposób.
Na koniec firma potrzebuje wydajności. Chociaż lepszy kod zwiększa wydajność przepisywania kodu tylko dlatego, że można go lepiej napisać, prawdopodobnie nie jest to najlepszy sposób na zwiększenie wartości produktu.
źródło