I have been reading Working Effectively with Legacy Code and Clean Code with the goal of learning strategies on how to begin cleaning up the existing code-base of a large ASP.NET webforms application.This system has been around since 2005 and since then has undergone a number of enhancements.  Originally the code was structured as follows (and is still largely structured this way):ASP.NET (aspx/ascx) Code-behind (c#)Business Logic Layer (c#) Data Access Layer (c#)  Database (Oracle)The main issue is that the code is procedural masquerading as object-oriented.  It virtually violates all of the guidelines described in both books.This is an example of a typical class in the Business Logic Layer:    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 >>}It has a lot of duplication, the class has a number of responsibilities, etc, etc - it is just generally 'un-clean' code.All of the code throughout the system is dependent on concrete implementations.  This is an example of a typical class in the Data Access Layer:    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 >>}This system was developed by me and a small team back in 2005 after a 1 week .NET course.  Before than my experience was in client-server applications.  Over the past 5 years I've come to recognise the benefits of automated unit testing, automated integration testing and automated acceptance testing (using Selenium or equivalent) but the current code-base seems impossible to introduce these concepts.We are now starting to work on a major enhancement project with tight time-frames.  The team consists of 5 .NET developers - 2 developers with a few years of .NET experience and 3 others with little or no .NET experience.  None of the team (including myself) has experience in using .NET unit testing or mocking frameworks.What strategy would you use to make this code cleaner, more object-oriented, testable and maintainable?
Convert from Procedural to Object Oriented Code
c#;.net;unit testing;object oriented
You mention two books in which one of the primary messages is The Boy Scout Rule i.e. clean up the code as you touch it. If you have a working system, a massive rewrite is counter-productive. Instead, as you add new functionality, make sure you improve the code as it stands. Write unit tests to cover the existing code that you need to change. Refactor that code so it is more pliable for change (making sure your tests still pass).Write tests for the new/revised functionalityWrite code to make the new tests passRefactor as necessary.To dive further in, Feathers talks about testing the application at its seams: the logical points at which the units connect. You can take advantage of a seam to create a stub or a mock for a dependency so that you can write tests around the dependent object. Let's take your AddressBO as an examplepublic 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);    }}There is an obvious seam between the AddressBO and the AddressDAO. Let's create an interface for the AddressDAO and allow the dependency to be injected into the AddressBO.public interface IAddressDAO{  TransferObject GetAddress(addressID);  //add other interface methods here.}public class AddressDAO:GenericDAO, IAddressDAO{  public TransferObject GetAddress(string addressID)  {    ///implementation goes here  }}Now you doctor up your AddressBO to allow for injectionpublic 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);    }}Here we're using poor man's dependency injection. Our only goal is to break the seam and allow us to test the AddressBO. Now in our unit tests we can make a mock IAddressDAO and validate the interactions between the two objects.