Konvertieren von prozeduralem zu objektorientiertem Code

16

Ich habe Effektiv mit Legacy-Code und Bereinigungscode arbeiten gelesen , um Strategien zu erlernen, wie die vorhandene Codebasis einer großen ASP.NET-Webforms-Anwendung bereinigt werden kann.

Dieses System gibt es seit 2005 und es wurden seitdem einige Verbesserungen vorgenommen. Ursprünglich war der Code wie folgt aufgebaut (und ist noch weitgehend so aufgebaut):

  • ASP.NET (aspx / ascx)
  • Code-Behind (c #)
  • Geschäftslogikschicht (c #)
  • Datenzugriffsschicht (c #)
  • Datenbank (Oracle)

Das Hauptproblem ist, dass der Code prozedural als objektorientiert maskiert wird. Es verstößt praktisch gegen alle in beiden Büchern beschriebenen Richtlinien.

Dies ist ein Beispiel für eine typische Klasse in der Geschäftslogikschicht:

    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 >>

}

Es gibt viele Duplikate, die Klasse hat eine Reihe von Verantwortlichkeiten usw. usw. - es handelt sich im Allgemeinen nur um "unreinen" Code.

Der gesamte Code im gesamten System hängt von konkreten Implementierungen ab.

Dies ist ein Beispiel für eine typische Klasse in der Datenzugriffsschicht:

    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 >>
}

Dieses System wurde 2005 von mir und einem kleinen Team nach einem einwöchigen .NET-Kurs entwickelt. Vorher war meine Erfahrung in Client-Server-Anwendungen. In den letzten 5 Jahren habe ich die Vorteile des automatisierten Komponententests, des automatisierten Integrationstests und des automatisierten Abnahmetests (mit Selen oder einem gleichwertigen Produkt) erkannt, aber die derzeitige Codebasis scheint es unmöglich zu sein, diese Konzepte einzuführen.

Wir fangen jetzt an, an einem großen Verbesserungsprojekt mit engen Zeitrahmen zu arbeiten. Das Team besteht aus 5 .NET-Entwicklern - 2 Entwicklern mit ein paar Jahren .NET-Erfahrung und 3 anderen mit wenig oder keiner .NET-Erfahrung. Keines der Teams (einschließlich meiner selbst) verfügt über Erfahrung in der Verwendung von .NET-Unit-Tests oder Spott-Frameworks.

Welche Strategie würden Sie anwenden, um diesen Code sauberer, objektorientierter, überprüfbarer und wartbarer zu machen?

Anthony
quelle
9
Im Übrigen kann es sich lohnen, zu überprüfen, ob ein Umschreiben des Systems mit Kosten verbunden ist. Der alte Code mag hässlich sein, aber wenn er gut genug funktioniert, kann es billiger sein, ihn mit groben Kanten zu versehen und Ihre Entwicklungszeit woanders zu investieren.
Smithco
Eine mögliche Rechtfertigung besteht darin, den Aufwand und die Kosten für manuelle Neuprüfungen nach jedem Erweiterungsprojekt zu reduzieren. Am Ende des letzten Projekts dauerte das manuelle Testen etwa 2 Monate. Wenn die Einführung automatisierterer Tests diesen Aufwand auf 1-2 Wochen reduziert, könnte sich dies lohnen.
Anthony
5
FÜR LEGACY CODE IST DIESES MATERIAL VERDAMMT GUT!
Job
Ich stimme zu, dass dies einigermaßen konsistent und strukturiert ist. Mein Hauptziel ist es, die Nebenwirkungen von Veränderungen zu reduzieren. Der Aufwand zum manuellen Testen der gesamten Anwendung nach (und während) jedem Projekt ist enorm. Ich hatte darüber nachgedacht, Selenium zum Testen über den Client zu verwenden. Ich habe eine Frage zu ServerFault ( serverfault.com/questions/236546/… ), um Vorschläge zum schnellen Zurücksetzen der Datenbank zu erhalten. Ich bin der Meinung, dass automatisierte Abnahmetests die meisten Vorteile bringen würden, ohne dass ein umfangreiches Umschreiben erforderlich wäre.
Anthony

Antworten:

16

Sie erwähnen zwei Bücher, in denen eine der Hauptbotschaften "The Boy Scout Rule" ist, dh bereinigen Sie den Code, wenn Sie ihn berühren. Wenn Sie ein funktionierendes System haben, ist eine massive Umschreibung kontraproduktiv. Stellen Sie stattdessen beim Hinzufügen neuer Funktionen sicher, dass Sie den Code so verbessern, wie er ist.

  • Schreiben Sie Komponententests, um den vorhandenen Code abzudecken, den Sie ändern müssen.
  • Überarbeiten Sie diesen Code, damit er flexibler für Änderungen ist (stellen Sie sicher, dass Ihre Tests weiterhin erfolgreich sind).
  • Schreiben Sie Tests für die neue / überarbeitete Funktionalität
  • Schreiben Sie Code, damit die neuen Tests bestanden werden
  • Refactor nach Bedarf.

Um weiter einzutauchen, spricht Feathers über das Testen der Anwendung aus der Nähe: die logischen Punkte, an denen sich die Einheiten verbinden. Sie können eine Naht nutzen, um einen Stub oder einen Mock für eine Abhängigkeit zu erstellen, sodass Sie Tests um das abhängige Objekt schreiben können. Nehmen wir als Beispiel Ihr AddressBO

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);
    }
}

Es gibt eine offensichtliche Naht zwischen dem AddressBO und dem AddressDAO. Erstellen wir eine Schnittstelle für das AddressDAO und lassen Sie die Abhängigkeit in das AddressBO einfließen.

public interface IAddressDAO
{
  TransferObject GetAddress(addressID);
  //add other interface methods here.
}

public class AddressDAO:GenericDAO, IAddressDAO
{
  public TransferObject GetAddress(string addressID)
  {
    ///implementation goes here
  }
}

Jetzt rupfen Sie Ihr AddressBO auf, um die Injektion zu ermöglichen

public class AddressBO
{
    private IAddressDAO _addressDAO;
    public AddressBO()
    {
      _addressDAO = new AddressDAO();
    }

    public AddressBO(IAddressDAO addressDAO)
    {
      _addressDAO = addressDAO;
    }

    public TransferObject GetAddress(string addressID)
    {
        if (StringUtils.IsNull(addressID))
        {
            throw new ValidationException("Address ID must be entered");
        }
        //call the injected AddressDAO
        return _addressDAO.GetAddress(addressID);
    }
}

Hier verwenden wir "die Abhängigkeitsinjektion des armen Mannes". Unser einziges Ziel ist es, die Naht zu durchbrechen und das AddressBO zu testen. Jetzt können wir in unseren Unit-Tests ein nachgemachtes IAddressDAO erstellen und die Interaktionen zwischen den beiden Objekten validieren.

Michael Brown
quelle
1
Ich stimme zu - ich mochte diese Strategie, wenn ich darüber las. Wir könnten Monate damit verbringen, den Code zu bereinigen, ohne wirklich einen Mehrwert zu schaffen. Wenn wir uns darauf konzentrieren, zu bereinigen, was wir ändern müssen, während wir eine neue Funktion hinzufügen, bekommen wir das Beste aus beiden Welten.
Anthony
Die einzige Herausforderung besteht darin, die Komponententests für den vorhandenen Code zu schreiben. Ich neige zuerst mehr zu Tests auf höherer Ebene, damit wir mit mehr Vertrauen Unit-Tests umgestalten und hinzufügen können.
Anthony
1
Ja, das Beste, was Sie tun können, ist Tests zu schreiben, mit denen überprüft wird, ob der Code das tut, was er tut. Sie können versuchen, Tests zu erstellen, die das korrekte Verhalten überprüfen. Es besteht jedoch die Gefahr, dass andere Funktionen beeinträchtigt werden, die nicht von Tests abgedeckt werden.
Michael Brown
Dies ist die beste Erklärung, die ich gesehen habe, um Feathers "find a seam" in die Praxis umzusetzen. Als jemand, der mehr mit Prozeduren als OO vertraut ist, gab es eine offensichtliche Naht zwischen dem AddressBO und dem AddressDAO , die für mich nicht offensichtlich war, aber dieses Beispiel hilft wirklich.
SeraM
5

Wenn ich mich recht erinnere effektiv mit Legacy Code zu arbeiten nicht garantieren, dass der neue Code besser ist als der alte (aus Sicht von Funktionalität / Fehlern). Die Refactorings in diesem Buch sind für das Beheben von Fehlern und das Hinzufügen neuer Funktionen gedacht.

Ein weiteres Buch, das ich empfehlen würde, ist die Brownfield-Anwendungsentwicklung in .NET, in der im Grunde gesagt wird, dass auch beim Neuschreiben keine vollständigen Änderungen vorgenommen werden sollen. Es geht darum, stetige, iterative Änderungen vorzunehmen, wenn Sie neue Funktionen hinzufügen oder Fehler beheben. Es geht auf die Kosten-Nutzen-Überlegungen ein und warnt davor, zu viel auf einmal abzubeißen. Während der effektiven Arbeit mit Legacy-Code geht es hauptsächlich um die Umgestaltung auf Mikro- / Code-Ebene. Brownfield-Anwendungsentwicklung in .NET hauptsächlich die Überlegungen auf höherer Ebene beim Re-Factoring (zusammen mit einigen Sachen auf Code-Ebene) behandelt.

Das Brownfield-Buch schlägt außerdem vor, herauszufinden, welcher Bereich des Codes Ihnen die meisten Probleme bereitet, und sich darauf zu konzentrieren. Alle anderen Bereiche, die nicht viel Wartung benötigen, sind möglicherweise keine Änderungen wert.

Matt
quelle
+1 für das Buch Brownfield Application Development in .Net
Gabriel Mongeon
Vielen Dank für die Buchempfehlung - ich werde es mir ansehen. Aus der Übersicht geht es mehr um .NET als um die beiden erwähnten Bücher, die sich anscheinend auf C, C ++ und Java konzentrieren.
Anthony
4

Für eine solche Legacy-App ist es wesentlich kostengünstiger, sie zunächst mit (automatisierten) Integrationstests auf höherer Ebene abzudecken als mit Unit-Tests. Mit den Integrationstests als Sicherheitsnetz können Sie dann in kleinen Schritten mit dem Refactoring beginnen, wenn es angebracht ist, dh wenn sich die Kosten für das Refactoring langfristig amortisieren. Wie andere angemerkt haben, ist dies nicht selbstverständlich.

Siehe auch meine frühere Antwort auf eine ähnliche Frage; Ich hoffe, Sie finden es nützlich.

Péter Török
quelle
Ich neige zunächst zu Tests auf höherer Ebene. Ich arbeite mit dem DBA zusammen, um herauszufinden, wie die Datenbank nach jeder Testausführung am besten wiederhergestellt werden kann.
Anthony
1

Seien Sie sehr vorsichtig, wenn Sie laufenden Code wegwerfen und neu schreiben ( Dinge, die Sie niemals tun sollten ). Sicher kann es hässlich sein, aber wenn es funktioniert, lass es sein. Siehe Joels Blog-Post, sicher schon über 10 Jahre alt, aber immer noch genau richtig.

Zachary K
quelle
Ich bin da nicht mit Joel einverstanden. Was er sagte, mag sich zu dieser Zeit relevant angefühlt haben, aber ist es nicht die Neufassung, die jetzt Mozilla Firefox heißt?
CashCow
1
Ja, aber es hat Netscape aus dem Geschäft gebracht! Es heißt nicht, dass es nie die richtige Wahl ist, von vorne zu beginnen, aber es ist etwas, bei dem man sehr vorsichtig sein muss. Und OO-Code ist nicht immer besser als prozeduraler Code.
Zachary K
1

Wie Mike sagte, ist die "Pfadfinder-Regel" hier wahrscheinlich die beste, wenn der Code funktioniert und keine ständige Quelle von Fehlerberichten ist. Ich würde es vorziehen, ihn dort stehen zu lassen und ihn im Laufe der Zeit langsam zu verbessern.

Lassen Sie während Ihres Erweiterungsprojekts neue Möglichkeiten zu. Verwenden Sie beispielsweise ein ORM für neue Funktionen und umgehen Sie das vorhandene Datenebenenmuster. Wenn Sie auf Verbesserungen stoßen, die vorhandenen Code berühren müssen, können Sie möglicherweise einen Teil des zugehörigen Codes auf die neue Art und Weise verschieben. Die Verwendung einer Fassade oder einiger Adapter an bestimmten Stellen kann Ihnen dabei helfen, den alten Code zu isolieren, möglicherweise sogar pro Ebene. Dies könnte Ihnen helfen, den alten Code durch die Zeit zu verhungern.

Dies kann Ihnen auch beim Hinzufügen von Komponententests helfen. Sie können mit dem neu erstellten Code beginnen und langsam einige Tests für alten Code hinzufügen, die Sie für die neuen Verbesserungen berühren müssen.

Joppe
quelle
1

Das sind beide gute Bücher. Wenn Sie damit beginnen, Code auf diese Weise umzuschreiben, ist es meines Erachtens wichtig, den Code auch mit Komponententests abzudecken, damit er beim Umschreiben stabil bleibt.

Dies muss in kleinen Schritten erledigt werden, und das Ändern dieser Art von Code kann das gesamte System leicht destabilisieren.

Ich würde keinen Code ändern, an dem Sie nicht aktiv arbeiten. Tun Sie dies nur für Code, den Sie aktiv verbessern oder korrigieren. Wenn etwas seinem Zweck dient, aber seit Jahren nicht mehr geändert wurde, lassen Sie es einfach. Es macht es, auch wenn Sie einen besseren Weg kennen.

Letztendlich braucht das Unternehmen Produktivität. Während besserer Code die Produktivität steigert, ist das Umschreiben von Code, nur weil er besser geschrieben werden könnte, wahrscheinlich nicht der beste Weg, um Ihrem Produkt einen Mehrwert zu verleihen.

Ehrenwerter Chow
quelle