Usage of Single Responsibility Principle with bank account services implementation

Question

I had code that violated the Single Responsibility Principle in a question on Stack Overflow.

In order to overcome that problem, I changed the code as follows.

1. Is this a good practice in order to overcome the problem?
2. Is this a pattern?
3. Is there a better way?
4. Does it satisfy the Unit Of Work pattern using LINQ-to-SQL?

namespace DomainObjectsForBank
{
    public interface IBankAccount
    {
        int BankAccountID { get; set; }
        string AccountStatus { get; set; }
        void FreezeAccount();
        RepositoryLayer.IRepository<RepositoryLayer.BankAccount> AccountRepository { get; set; }
    }

    public class FixedBankAccount : IBankAccount
    {
        public int BankAccountID { get; set; }
        public string AccountStatus { get; set; }

        public void FreezeAccount()
        {
            //ChangeAccountStatus();
            AccountStatus = "Frozen";
        }

        //private void ChangeAccountStatus()
        //{
        //    AccountStatus = "Frozen";
        //    RepositoryLayer.BankAccount repositoryBankAccEntity = new RepositoryLayer.BankAccount();
        //    repositoryBankAccEntity.BankAccountID = this.BankAccountID;
        //    accountRepository.UpdateChangesByAttach(repositoryBankAccEntity);
        //    repositoryBankAccEntity.Status = "Frozen";
        //    accountRepository.SubmitChanges();
        //}

        private RepositoryLayer.IRepository<RepositoryLayer.BankAccount> accountRepository;
        public RepositoryLayer.IRepository<RepositoryLayer.BankAccount> AccountRepository
        {
            get
            {
                return accountRepository;
            }
            set
            {
                accountRepository = value;
            }
        }
    }
}

---

using System.Collections.Generic;
using System;

namespace ApplicationServiceForBank
{
    public class BankAccountService
    {
        RepositoryLayer.IRepository<RepositoryLayer.BankAccount> accountRepository;
        ApplicationServiceForBank.IBankAccountFactory bankFactory;

        public BankAccountService(RepositoryLayer.IRepository<RepositoryLayer.BankAccount> repo, IBankAccountFactory bankFact)
        {
            accountRepository = repo;
            bankFactory = bankFact;
        }

        public void FreezeAllAccountsForUser(int userId)
        {
            IEnumerable<RepositoryLayer.BankAccount> accountsForUser = accountRepository.FindAll(p => p.BankUser.UserID == userId);
            foreach (RepositoryLayer.BankAccount oneOfRepositroyAccounts in accountsForUser)
            {
                DomainObjectsForBank.IBankAccount domainBankAccountObj = bankFactory.CreateAccount(oneOfRepositroyAccounts);
                if (domainBankAccountObj != null)
                {
                    domainBankAccountObj.BankAccountID = oneOfRepositroyAccounts.BankAccountID;
                    domainBankAccountObj.FreezeAccount();

                    this.accountRepository.UpdateChangesByAttach(oneOfRepositroyAccounts);
                    oneOfRepositroyAccounts.Status = domainBankAccountObj.AccountStatus;
                    this.accountRepository.SubmitChanges();
                }
            }
        }
    }

    public interface IBankAccountFactory
    {
        DomainObjectsForBank.IBankAccount CreateAccount(RepositoryLayer.BankAccount repositroyAccount);
    }

    public class MySimpleBankAccountFactory : IBankAccountFactory
    {
        //Is it correct to accept repository inside factory?
        public DomainObjectsForBank.IBankAccount CreateAccount(RepositoryLayer.BankAccount repositroyAccount)
        {
            DomainObjectsForBank.IBankAccount acc = null;

            if (String.Equals(repositroyAccount.AccountType, "Fixed"))
            {
                acc = new DomainObjectsForBank.FixedBankAccount();
            }

            if (String.Equals(repositroyAccount.AccountType, "Savings"))
            {
                //acc = new DomainObjectsForBank.SavingsBankAccount();
            }

            return acc;
        }
    }
}

Answer 1

I'm not sure about some of your questions, but a few ideas I have are:

1. Could you make the the account status and even account types an enumeration? I tend to try and err on the use of enumerations over string literals. I believe the latest version of Linq to SQL supports this?
2. Why do you need to expose the repository on your IBankAccount interface. Why would the interface even care about a repository? I think this is better being supplied to any concrete classes via it's constructor? It would also make unit testing easier I would think.
3. I personally wouldn't pass in the whole BankAccount object into the factory method. It only needs the account type so I would pass in that. Probably I would look at using a switch or if else in that factory method as well and throwing an exception if you can't create it (depends on your requirements of this method).

Just a few thoughts.