patterncsharpMinor
Factory Method Pattern - Base implementation to Add New Records
Viewed 0 times
newmethodfactoryrecordsimplementationpatternaddbase
Problem
Most importantly which bothers me in the following implementation is the call in
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
Program (main call)
Making a call to add a new
AddManagerBase (abstract)
Concrete classes which needs to Add new records into the system should inherit from this
```
abstract class AddManagerBase 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 DatabaseOpera
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 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 DatabaseOpera
Solution
-
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.
-
This variable can be made
-
You can use interpolated string here (C# 6 feature):
Like this :
You can just do :
Well the same can happen here :
Right ? You cant just forget to add essential part of your implementation. Calling
What you can do to make it a little bit more easier to remember I guess, is to make
And in your derived class :
The cool thing here is that right now the derived class is adding nothing to the already existing functionality of
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.
-
This variable can be made
readonly as you are assigning value to it only in the constructor.private TransactionModel _transaction-
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}");
}- 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);
}- 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.Code Snippets
private TransactionModel _transactionpublic static void AuditLog(AuditType auditType, int transactionId, DateTimeOffset date)
{
Console.WriteLine(string.Format("{0} {1} {2}", auditType, transactionId, date));
}public static void AuditLog(AuditType auditType, int transactionId, DateTimeOffset date)
{
Console.WriteLine($"{auditType} {transactionId} {date}");
}protected override void DatabaseOperations()
{
//..
base.SetTransactionId(newBranchSqlIdentityId);
}protected override void DatabaseOperations()
{
//..
SetTransactionId(newBranchSqlIdentityId);
}Context
StackExchange Code Review Q#152304, answer score: 3
Revisions (0)
No revisions yet.