I have a self-made project called "Library Management", I have many forms: Book Form, Book Type Form, Author Form,... But I realized that the code in the forms is very similar (only in some places like controls, ..). How to reduce this duplication?. I know I have to create a base class and forms will inherit this class but I don't know what to write in that base class. Please help me, thank you very much
private void AuthorForm_Load(object sender, EventArgs e)
{
if (_authorDAL == null)
_authorDAL = new AuthorDAL();
loadData().ContinueWith((t) =>
{
if (InvokeRequired)
{
Invoke((MethodInvoker)(() =>
{
bindingData();
applyUIStrings();
}));
}
else
{
bindingData();
applyUIStrings();
}
});
}
And this is the duplicate code:
private void BookTypeForm_Load(object sender, EventArgs e)
{
if (bookTypeDAL == null)
bookTypeDAL = new BookTypeDAL();
loadData().ContinueWith((t) =>
{
if (InvokeRequired)
{
Invoke((MethodInvoker)(() =>
{
bindingData();
applyUIStrings();
}));
}
else
{
bindingData();
applyUIStrings();
}
});
}
CodePudding user response:
There are a number of approaches you could take, but with inheritance, you could create an abstract base class:
public abstract BaseForm : Form
{
protected virtual void CreateDALObject();
protected virtual void BindingData();
protected virtual void ApplyUIStrings();
protected void Form_Load(object sender, EventArgs e)
{
CreateDALObject();
loadData().ContinueWith((t) =>
{
if (InvokeRequired)
{
Invoke((MethodInvoker)(() =>
{
BindingData();
ApplyUIStrings();
}));
}
else
{
BindingData();
ApplyUIStrings();
}
});
}
}
Which can then be derived from like this:
public class AuthorForm : BaseForm
{
// ...
protected override void CreateDALObject()
{
if (_authorDAL == null)
_authorDAL = new AuthorDAL();
}
// ...
}
Or you could try to move methods out into supporting classes, which can be preferable to using inheritance, but sometimes difficult because WinForms so heavily rely on inheritance.
Edit note: I've removed the generic example I originally gave, because it would have required passing the objects to be initialised by reference, which wouldn't be a clean approach.
CodePudding user response:
Just change the name to just Load() in your base class
Make something like this:\
class BaseClass{
private void Load(object sender, EventArgs e)
{
if (_authorDAL == null)
_authorDAL = new AuthorDAL();
loadData().ContinueWith((t) =>
{
if (InvokeRequired)
{
Invoke((MethodInvoker)(() =>
{
bindingData();
applyUIStrings();
}));
}
else
{
bindingData();
applyUIStrings();
}
});
}
}
class OtherClass : BaseClass{
//other attributes/methods
}
CodePudding user response:
Unfortunately your code has more problems than just duplicate code. This makes it hard to change and hard to explain in textual format. Since we don't know about the dependencies of the methods bindingdata()
and applyUIStrings()
, it's hard to tell what the result after a refactoring exactly looks like and what problems your code will have next. This is better explained with the complete real code in a pair programming style.
The problems:
The method is doing more than one thing: it updates the data in the business model (
bindingdata()
) and it updates the UI (applyUIStrings()
). IMHO you should fix this in one place first.Due to FCoI (favor composition over inheritance), I would not suggest a base class. You might just be moving the business logic into the base class and thus couple business logic and derived UIs closely together.
DRY (don't repeat yourself), as you notices
SRP (single reponsibility principle), because your UI class is responsible for too many things.
Later, review the whole code for all 5 SOLID principles.
Refactoring:
You can refactor the part of the methods that is identical. Use a tool supported refactoring and use the refactoring's name is "Extract method".
Next, there's a "Move" refactoring as well to move code into a different class. This won't be easy, in fact quite hard, I guess, because you have a lot of dependencies to code that runs in the UI. Hard to tell without seeing the rest of the code.