Factory Method Pattern - Base implementation to Add New Records

Question

Most importantly which bothers me in the following implementation is the call in BranchAddManager, do I really need to call base.StartAddNewOperation();? How can I get away without making this call?

public override void AddNew()
    {
        //I would like to avoid calling the base method, if I forget then I may run into bug.
        base.StartAddNewOperation();
    }

Please help me to verify whether this approach make sense to you? Am also open to take another approach, if you think it make more sense. I have just noticed, this could be a builder pattern approach too. It will be a bit too much to add the entire code here but I try to demonstrate with relevant code, if you think the explanation is not enough then please let me know.

Program (main call)

Making a call to add a new Branch Record by setting properties to BranchModel.

class Program
    {
        static void Main(string[] args)
        {
            var model = new BranchModel()
            {
                BranchName = "Test Branch",
                Description = "The description of a branch",
                BranchTypeId = 2
            };

            try
            {
                new BranchAddManager(model).AddNew();
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
            }
        }
    }

AddManagerBase (abstract)

Concrete classes which needs to Add new records into the system should inherit from this base class.

abstract class AddManagerBase<T> where T : class
{
    private TransactionModel _transaction;
    protected readonly T _model;

    public AddManagerBase(T model)
    {
        _model = model;

        _transaction = new TransactionModel()
        {
            TransactionDate = DateTimeOffset.Now
        };
    }

    public abstract void AddNew();
    protected abstract void RunValidations();
    protected abstract void PreDatabaseOperation();
    protected abstract void DatabaseOperations();

    protected void StartAddNewOperation()
    {
        RunValidations();
        PreDatabaseOperation();
        DatabaseOperations();
        LogThisActivity();
    }

    protected void SetTransactionId(int transactionId)
    {
        Console.WriteLine("SetTransactionId()");
        _transaction.TransactionId = transactionId;
    }

    private void LogThisActivity()
    {
        Logger.AuditLog(AuditType.Add, _transaction.TransactionId, _transaction.TransactionDate);
    }
}

BranchAddManager (concrete)

This class inherits the base class. The following example is supposed to add a new Branch Record.

class BranchAddManager : AddManagerBase<BranchModel>
{
    public BranchAddManager(BranchModel model) : base(model)
    { }

    public override void AddNew()
    {
        //I would like to avoid calling the base method, if I forget then I may run into bug.
        base.StartAddNewOperation();
    }

    protected override void RunValidations()
    {
        //In this example ensure the BranchTypeId exists in the DB.
        Console.WriteLine("RunValidations()");
    }

    protected override void PreDatabaseOperation()
    {
        //Convert the Model to Pojo/Poco, etc..
        Console.WriteLine("PreDatabaseOperation()");
    }

    protected override void DatabaseOperations()
    {
        //Insert into Branch Table
        Console.WriteLine("DatabaseOperations()");

        //manually setting the ID for this example but usually this is given by the Database when the new record gets inserted.
        int newBranchSqlIdentityId = 9990;

        base.SetTransactionId(newBranchSqlIdentityId);
    }
}

Logger, AuditType, BranchModel and TransactionModel (supporting classes)

public enum AuditType
    {
        Add,
        Delete,
        Update
    }

    public class Logger
    {
        public static void AuditLog(AuditType auditType, int transactionId, DateTimeOffset date)
        {
            Console.WriteLine(string.Format("{0} {1} {2}", auditType, transactionId, date));
        }
    }

class TransactionModel
    {
        public int TransactionId { get; set; }
        public DateTimeOffset TransactionDate { get; set; }
    }

class BranchModel
    {
        public string BranchName { get; set; }
        public string Description { get; set; }
        public int BranchTypeId { get; set; }
    }

Answer 1

There is no factory method pattern. Factory method means that a class has a static method (or multiple) for creating an instance of itself usually having a private constructor to prevent creating it in a normal way:

class Foo
{
    private Foo(..) {}
    public static Foo Create(..) => new Foo(..);
}

---

What you do is a simple inheritance that needs a few improvements. You should start by renaming the base class and the models. Classes are objects and should have noun names. How about ModelService?

The models don't need the Model suffix and their properties don't need to be prefixed with the model name e.g.:

class Transaction
{
    public int Id { get; set; }
    public DateTimeOffset Date { get; set; }
}

---

Don't you think that currently AddManagerBase is not a very useful class?. You create an instance with a concrete model and all you can do is to repetedly call the AddNew method. Shouldn't the AddNew (or better Add) require a model to add? What happens if you call it again with the same model? Is it still new?

You can fix it by changing the Add method to require a model. At the same time you can remove the StartAddNewOperation and put its content inside the Add method. You won't need to override this one any more. It calls the other methods in the right order allowing you to implement them and do something with the model along the way.

abstract class ModelService<T> where T : class
{
    public void Add(T model)
    {
        var transaction = new Transaction
        {
            Date = DateTimeOffset.Now
        };
        RunValidations(model);
        PreDatabaseOperation(model);
        DatabaseOperation(model, transaction);
        LogActivity(AuditType.Add, transaction);
    }
    protected abstract void RunValidations(T model);
    protected abstract void PreDatabaseOperation(T model);
    protected abstract void DatabaseOperation(T model, Transaction transaction);

    private void LogActivity(AuditType auditType, Transaction transaction)
    {
        Logger.AuditLog(
            auditType,
            transaction.Id,
            transaction.Date
        );
    }
}

---

Implementing the BranchService is now much easier because you no longer have to care about the order of operations and override the Add method. Just do the validation and whatever else it requires by overriding the other methods.

The new implementation also no longer needs a custom constructor because the new Add method requires a parameter.

class BranchService : ModelService<Branch>
{        
    protected override void RunValidations(Branch branch)
    {
        //In this example ensure the BranchTypeId exists in the DB.
        Console.WriteLine("RunValidations()");
    }

    protected override void PreDatabaseOperation(Branch branch)
    {
        //Convert the Model to Pojo/Poco, etc..
        Console.WriteLine("PreDatabaseOperation()");
    }

    protected override void DatabaseOperations(Branch branch, Transaction transaction)
    {
        //Insert into Branch Table
        Console.WriteLine("DatabaseOperations()");

        //manually setting the ID for this example but usually this is given by the Database when the new record gets inserted.
        transaction.Id = 9990;
    }
}

Answer 2

1. It doesn't makes sense to have a public constructor in an abstract class, since abstract classes are instantiated through they're derived types, more appropriate access modifier would be protected.

2. This variable can be made readonly as you are assigning value to it only in the constructor.

   private TransactionModel _transaction
   

3. You can use interpolated string here (C# 6 feature):

public static void AuditLog(AuditType auditType, int transactionId, DateTimeOffset date)
{
    Console.WriteLine(string.Format("{0} {1} {2}", auditType, transactionId, date));
}

Like this :

public static void AuditLog(AuditType auditType, int transactionId, DateTimeOffset date)
{
    Console.WriteLine($"{auditType} {transactionId} {date}");
}

4. There is no need of specifying explicitly that you are calling some method in the base class if there is no way to change the functionality of that method :

protected override void DatabaseOperations()
{
    //..
    base.SetTransactionId(newBranchSqlIdentityId);
}

You can just do :

protected override void DatabaseOperations()
{
    //..
    SetTransactionId(newBranchSqlIdentityId);
}

5. You are saying you want to avoid calling the base class here because you might forget to do so :

public override void AddNew()
{
    //I would like to avoid calling the base method, if I forget then I may run into bug.
    base.StartAddNewOperation();
}

Well the same can happen here :

protected override void DatabaseOperations()
{
    //Insert into Branch Table
    Console.WriteLine("DatabaseOperations()");

    //manually setting the ID for this example but usually this is given by the Database when the new record gets inserted.
    int newBranchSqlIdentityId = 9990;

    base.SetTransactionId(newBranchSqlIdentityId);
}

Right ? You cant just forget to add essential part of your implementation. Calling Console.WriteLine() is no different than calling base.StartAddNewOperation();. You can forget that too !

What you can do to make it a little bit more easier to remember I guess, is to make AddNew virtual, if you are always going to call base.StartAddNewOperation(); in it, this way whenever you are adding new functionality you can just refer to the same method but just held in the base class.

public virtual void AddNew()
{
    StartAddNewOperation();
}

And in your derived class :

public override void AddNew()
{
    //I would like to avoid calling the base method, if I forget then I may run into bug.
    base.AddNew();
}

The cool thing here is that right now the derived class is adding nothing to the already existing functionality of AddNew() so you can just remove the whole method in the derived class (unless you want to add extra logic in there), you will still be able to call AddNew() but it will instead access the base implementation.

Answer 3

In your base class

// rename StartAddNewOperation
public void AddNew()
{ 
    // call your base methods as normal.
    AddNewInternal(); // name whatever you want,  could even be an event called OnAddNew or whatever
}

protected abstract void AddNewInternal();

And in your derived class just override AddNewInternal. No need to call base class anymore.

Answer 4

If you want to get away Addnew method you can do the following

abstract class AddManagerBase<T> where T : class
{
...
    public void StartAddNewOperation()
    {
       RunValidations();
       PreDatabaseOperation();
       DatabaseOperations();
       LogThisActivity();
    }
...
}

And BranchAddManager class will not have AddNew method
Your client code will look like

 try
    {
        new BranchAddManager(model).StartAddNewOperation();
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
    }

By the way i don't see factory method pattern as you mentioned in the topic. There is template method pattern. /Please correct me if i am wrong/