mardi 3 octobre 2017

Refactoring code and writing unit tests

I am trying to refactor below code applying design patterns and trying to write tests. I have identified below code smells in this please suggest me if I am wrong or if anything I am missing.

  public class AccountDataStore
{
    public Account GetAccount(string accountNumber)
    {
        // Access database to retrieve account, code removed for brevity 
        return new Account();
    }

    public void UpdateAccount(Account account)
    {
        // Update account in database, code removed for brevity
    }
}

public class BackupAccountDataStore
{
    public Account GetAccount(string accountNumber)
    {
        // Access backup data base to retrieve account, code removed for brevity 
        return new Account();
    }

    public void UpdateAccount(Account account)
    {
        // Update account in backup database, code removed for brevity
    }
}


 public class PaymentService : IPaymentService
{
    public MakePaymentResult MakePayment(MakePaymentRequest request)
    {
        var dataStoreType = ConfigurationManager.AppSettings["DataStoreType"];

        Account account = null;

        if (dataStoreType == "Backup")
        {
            var accountDataStore = new BackupAccountDataStore();
            account = accountDataStore.GetAccount(request.DebtorAccountNumber);
        }
        else
        {
            var accountDataStore = new AccountDataStore();
            account = accountDataStore.GetAccount(request.DebtorAccountNumber);
        }

        var result = new MakePaymentResult();

        switch (request.PaymentScheme)
        {
            case PaymentScheme.Bacs:
                if (account == null)
                {
                    result.Success = false;
                }
                else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.Bacs))
                {
                    result.Success = false;
                }
                break;

            case PaymentScheme.FasterPayments:
                if (account == null)
                {
                    result.Success = false;
                }
                else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.FasterPayments))
                {
                    result.Success = false;
                }
                else if (account.Balance < request.Amount)
                {
                    result.Success = false;
                }
                break;

            case PaymentScheme.Chaps:
                if (account == null)
                {
                    result.Success = false;
                }
                else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.Chaps))
                {
                    result.Success = false;
                }
                else if (account.Status != AccountStatus.Live)
                {
                    result.Success = false;
                }
                break;
        }

        if (result.Success)
        {
            account.Balance -= request.Amount;

            if (dataStoreType == "Backup")
            {
                var accountDataStore = new BackupAccountDataStore();
                accountDataStore.UpdateAccount(account);
            }
            else
            {
                var accountDataStore = new AccountDataStore();
                accountDataStore.UpdateAccount(account);
            }
        }

        return result;
    }
}

First change: I have extracted configuration and created a folder cofig and then created an IConfig interface and then an class implementing it, so I can more config settings in future if needed.

public interface IConfiguration
{
    string DataStoreType { get; }
}
public class Configuration : IConfiguration
{
    string IConfiguration.DataStoreType => ConfigurationManager.AppSettings["DataStoreType"];
}

Second Change: BackupAccountDataStore and AcccountDataStore are doing same thing AddAccount and Update so I have created an Interface so that these classes can implement it DRY don't repeat yourself

 public interface IAccountDataStore
{
    Account GetAccount(string accountNumber);

    void UpdateAccount(Account account);
}

Third change: Too many switch statements removing them and implementing polymorphism, I have added classes instead of using switch statements

  public class PaymentSchemeBacs:MakePaymentResult
{
    public override bool Success( Account account)
    {
        if (account == null)
        {
            return false;
        }
        if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.Bacs))
        {
            return false;
        }
        return true;
    }
}

Please kindly suggest me if anything needs to be change with in this code.

Aucun commentaire:

Enregistrer un commentaire