patterncsharpMinor
Click handlers for two to ten buttons
Viewed 0 times
clickhandlerstenbuttonstwofor
Problem
I have click event handlers for two buttons. The code is working well, but I want to apply the DRY principle. How should I rewrite the following code in the button_click event handler so that I don't have to repeat in all my 10 buttons?
private void CreateWindows(UserControl ux, Form rpt, object sender, ItemClickEventArgs e)
{
ux.Show();
rpt.SuspendLayout();
rpt.ControlBox = false;
rpt.TopLevel = false;
rpt.WindowState = FormWindowState.Maximized;
rpt.MdiParent = this;
rpt.Show();
rpt.ResumeLayout();
dock1.SuspendLayout();
dock1.Controls.Clear();
dock1.Controls.Add(ux);
dock1.ResumeLayout();
}
private BaseDocument FindDocument() where T : Form
{
foreach (BaseDocument document in dxDocumentManager.View.Documents) {
if (document.Form is T) {
return document;
}
}
return null;
}
private void button1_ItemClick(object sender, ItemClickEventArgs e)
{
dxRpt12MonthsItemPurchaseAnalysisByAmount filter = new formFilter1();
BaseDocument document = FindDocument();
if(document != null)
{
tabbedView1.Controller.Activate(document);
if (this.ActiveMdiChild is formReport1)
{
filter.Focus();
}
}
else
{
CreateWindows(new formFilter1(), new formReport1(), sender, e);
}
}
private void button2_ItemClick(object sender, ItemClickEventArgs e)
{
fmSalesInvoiceSearch filter = new formFilter2();
BaseDocument document = FindDocument();
if (document != null)
{
tabbedView1.Controller.Activate(document);
if (this.ActiveMdiChild is formReport2)
{
filter.Focus();
}
}
else
{
CreateWindows(new formFilter2(), new formReport2(), sender, e);
}
}Solution
Naming:
Like the previous answer stated, your classnames are far from following conventions. The should be
Tip: Names of Classes, Structs, and Interfaces
Braces:
Microsoft doesn't have any conventions on the formatting of your braces. This means you can chose between following:
and
But it's best practice to stick with one and not to mix the two up in your code. (I prefer the second use of braces.)
Logical use of variables:
In both click event handlers you instantiate a new filter. Whether or not
Generic method:
First of all, the following code is written out of my head and not tested as I have little information what all your classes are/mean. But here's an attempt to make it generic:
And the usage of the method:
Notes:
Tip: new Constraint (C# Reference)
I cannot reproduce your code and therefore cannot test the method. Please let me know if you got it to work using this method. Hope this helps!
Like the previous answer stated, your classnames are far from following conventions. The should be
PascalCase and not camelCase. Also a name like dxRpt12MonthsItemPurchaseAnalysisByAmount is definitly not meaningful, not for you, not for others who read your code. Try names like FirstReportForm, FirstFilterForm or FilterForm1, ReportForm1 and so on.Tip: Names of Classes, Structs, and Interfaces
Braces:
Microsoft doesn't have any conventions on the formatting of your braces. This means you can chose between following:
if (isValid) {
DoSomething();
}and
if (isValid)
{
DoSomething();
}But it's best practice to stick with one and not to mix the two up in your code. (I prefer the second use of braces.)
Logical use of variables:
In both click event handlers you instantiate a new filter. Whether or not
document is null, you call CreateWindows with again a new instance of that filter-class. Just re-use the instance you created before. You didn't manipulate it in any way.Generic method:
First of all, the following code is written out of my head and not tested as I have little information what all your classes are/mean. But here's an attempt to make it generic:
private void HandleButtonClick(Form filterForm) where T : Form, new()
{
var document = FindDocument();
if(document == null)
{
CreateWindows(filterForm, new T(), sender, e);
}
else
{
tabbedView1.Controller.Activate(document);
if (this.ActiveMdiChild is T)
{
filterForm.Focus();
}
}
}And the usage of the method:
private void button1_ItemClick(object sender, ItemClickEventArgs e)
{
HandleButtonClick(new FilterForm1());
}
private void button2_ItemClick(object sender, ItemClickEventArgs e)
{
HandleButtonClick(new FilterForm2());
}Notes:
Tis the type of the report form
- The
new()constraint is set on the method, check tip for more info
- I reversed the null check for readability, this might be personal choice
Tip: new Constraint (C# Reference)
I cannot reproduce your code and therefore cannot test the method. Please let me know if you got it to work using this method. Hope this helps!
Code Snippets
if (isValid) {
DoSomething();
}if (isValid)
{
DoSomething();
}private void HandleButtonClick<T>(Form filterForm) where T : Form, new()
{
var document = FindDocument<T>();
if(document == null)
{
CreateWindows(filterForm, new T(), sender, e);
}
else
{
tabbedView1.Controller.Activate(document);
if (this.ActiveMdiChild is T)
{
filterForm.Focus();
}
}
}private void button1_ItemClick(object sender, ItemClickEventArgs e)
{
HandleButtonClick<ReportForm1>(new FilterForm1());
}
private void button2_ItemClick(object sender, ItemClickEventArgs e)
{
HandleButtonClick<ReportForm2>(new FilterForm2());
}Context
StackExchange Code Review Q#70077, answer score: 3
Revisions (0)
No revisions yet.