3
\$\begingroup\$

I'm currently reading on how to implement design patterns. I'm trying to implement the design patterns but need help on whether my understanding is correct.

I have a windows form in C# project that keeps some information. I created a bunch of TextBox and ComboBox dynamically, depend upon user input.

For each component I have created a class, each with its own responsibility.

After reading about abstraction, the following is my understanding:

An abstract class is a partially defined class that cannot be instantiated.

The purpose of an abstract class is to define some common behavior that can be inherited by multiple subclasses.

An abstract class should be used when there is IS-A relationship.

Abstract classes are also useful in the case of modifications to the project. If you plan on updating the base class in your project, it is better to make the class abstract. Because you can define a functionality in an abstract base class and automatically all the inheriting classes will have the same functionality without disturbing the hierarchy.

After reading about abstraction I implemented the abstract class below, with the following rationale:

Here I need to create multiple class where all the class will have the same behavior. So I thought of creating a Component abstract class.

All the subclass which is also a form of component will have a is-a relationship with the main class.

On whenever I create a new dynamic component I can just inherit the add_components class so that the new class will implement the add_dynamic_components methods.

// abstract class
public abstract class add_components
{
    public abstract void add_dynamic_components(int noOfComponets, int locationX, int locationY, Form1 l, int currentTabIndex);
}

The classes that will inherit the above abstract classes are below. This classes are example I have nearly 8 classes which inherit the abstract class.

public class addDynamicCptboxComponents : add_components
{
    public override void add_dynamic_components(int getNoOfTxtBox, int pointX, int pointY, Form1 f, int currentTabIndex)
    {
        TextBox txtBox = new TextBox();
        f.panel1.Controls.Add(txtBox);
        txtBox.Location = new Point(pointX, pointY);
        txtBox.Size = new System.Drawing.Size(75, 23);
        f.panel1.Controls.Add(txtBox);
        txtBox.Name = "Add_txtBox" + getNoOfTxtBox;
        txtBox.TabIndex = currentTabIndex * 7 + 2;
    }
}

public class addDynamicDateofServiceComponents : add_components
{

    public override void add_dynamic_components(int getNoOfTxtBox, int pointX, int pointY, Form1 f, int currentTabIndex)
    {
        TextBox txtBox = new TextBox();
        f.panel1.Controls.Add(txtBox);
        txtBox.Location = new Point(pointX, pointY);
        txtBox.Size = new System.Drawing.Size(75, 23);
        f.panel1.Controls.Add(txtBox);
        txtBox.Name = "Add_dos_txtBox" + getNoOfTxtBox;
        txtBox.TabIndex = currentTabIndex * 7 + 1;
    }
}
public class addDynamicSpellboxComponents : add_components
{

    public override void add_dynamic_components(int getNoOfSpellTxtBox, int spPointX, int spPointY, Form1 f, int currentTabIndex)
    {
        RichTextBox sp = new RichTextBox();
        f.panel1.Controls.Add(sp);
        sp.Location = new Point(spPointX, spPointY);
        sp.Size = new System.Drawing.Size(230, 43);
        f.panel1.Controls.Add(sp);
        sp.Name = "RichTextbox" + getNoOfSpellTxtBox;
        sp.TabIndex = currentTabIndex * 7 + 3;

    }
}
public class addDynamicComboboxxComponents : add_components
{

    public override void add_dynamic_components(int getNoOfComboboxValue, int cmbPointX, int cmbPointY, Form1 f, int currentTabIndex)
    {
        ComboBox cmb = new ComboBox();
        f.panel1.Controls.Add(cmb);
        cmb.Location = new Point(cmbPointX, cmbPointY);
        cmb.Size = new System.Drawing.Size(95, 23);
        cmb.Name = "Add_combobox" + getNoOfComboboxValue;
        addItems(cmb);
        cmb.DropDownStyle = ComboBoxStyle.DropDownList;

        cmb.SelectedIndexChanged += new EventHandler(f.cmbBoxValueChange);
        cmb.TabIndex = currentTabIndex * 7 + 4;
    }

    public void addItems(ComboBox addClaimlinestatusCombobox)
    {
        addClaimlinestatusCombobox.Items.Add("Finding");
        addClaimlinestatusCombobox.Items.Add("No Finding");
        addClaimlinestatusCombobox.Items.Add("InComplete");
        addClaimlinestatusCombobox.Items.Add("Skipped");
    }

}
public class addDynamicProviderCptComponents : add_components
{
    public override void add_dynamic_components(int getNoOfTxtBox, int pncPointX, int pncPointY, Form1 f, int currentTabIndex)
    {
        SpellBox providercpttxtBox = new SpellBox();
        f.panel1.Controls.Add(providercpttxtBox);
        providercpttxtBox.Location = new Point(pncPointX, pncPointY);
        providercpttxtBox.Size = new System.Drawing.Size(238, 23);
        f.panel1.Controls.Add(providercpttxtBox);
        providercpttxtBox.Name = "Add_Providr_txtBox" + getNoOfTxtBox;
        providercpttxtBox.Enabled = false;
        providercpttxtBox.TabIndex = currentTabIndex * 7 + 7;
    }
}
public class addDynamiContributorFindingsComboboxComponents : add_components
{
    public override void add_dynamic_components(int getNoOfComboboxValue, int finalcmbPointX, int finalcmbPointY, Form1 f, int currentTabIndex)
    {
        ComboBox cmb = new ComboBox();
        f.panel1.Controls.Add(cmb);
        cmb.Location = new Point(finalcmbPointX, finalcmbPointY);
        cmb.Size = new System.Drawing.Size(110, 23);
        cmb.Name = "Add_contributor_combobox" + getNoOfComboboxValue;
        addItems(cmb);
        cmb.DropDownStyle = ComboBoxStyle.DropDownList;
        cmb.Enabled = false;
        cmb.TabIndex = currentTabIndex * 7 + 6;
        //  cmb.SelectedIndexChanged += new EventHandler(f.cmbBoxValueChange);
    }
    public void addItems(ComboBox cmb1)
    {
        cmb1.Items.Add("History");
        cmb1.Items.Add("History,MDM");
        cmb1.Items.Add("History,Exam");
        cmb1.Items.Add("History,Exam,MDM");
        cmb1.Items.Add("Exam");
        cmb1.Items.Add("Exam,MDM");
        cmb1.Items.Add("MDM");
        cmb1.Items.Add("RRR");
        cmb1.Items.Add("Time");
        cmb1.Items.Add("Established patient");
        cmb1.Items.Add("Inpatient");
        cmb1.Items.Add("Insufficient documentation");
        cmb1.Items.Add("Wrong E/M Category");
    }
}
class addCptRecommendedTxtBox : add_components
{
    public override void add_dynamic_components(int getNoOfTxtBox, int txtboxRecommendedpointX, int txtboxRecommendedpointY, Form1 f, int currentTabIndex)
    {
        TextBox txtBox = new TextBox();
        f.panel1.Controls.Add(txtBox);
        txtBox.Location = new Point(txtboxRecommendedpointX, txtboxRecommendedpointY);
        txtBox.Size = new System.Drawing.Size(100, 23);
        f.panel1.Controls.Add(txtBox);
        txtBox.Name = "Add_recommendedcpt_txtBox" + getNoOfTxtBox;
        txtBox.Enabled = false;
        txtBox.TabIndex = currentTabIndex * 7 + 5;

    }
}
class addButtonComponent : add_components
{
    public override void add_dynamic_components(int getNoOfButton, int btnPointX, int btnPointY, Form1 f, int currentTabIndex)
    {
        Button btn = new Button();
        f.panel1.Controls.Add(btn);
        btn.Location = new Point(btnPointX, btnPointY);
        btn.Size = new System.Drawing.Size(75, 23);
        f.panel1.Controls.Add(btn);
        btn.Name = "Add_Button" + getNoOfButton;
        btn.Text = "Add";
        btn.TabIndex = currentTabIndex * 7 + 8;
        btn.Click += new EventHandler(f.btnAddCptInformation);

    }
}

So now my questions are

  1. Whether my thought process on abstraction is correct?

  2. Even though I have created abstract class on implementing I'm just calling the subclass instances to call the method since abstract class cannot be instantiated. Here am I missing something. The reason for this question is, whether we only use abstract class for better reading purpose or there is any special classes where it will be useful. Most of the examples in the internet says the abstract helps in re-usability.

  3. This is how I implemented the class in form. Here I don't think I'm leveraging the abstract concept. The below code is an excerpt which deals with the abstract class

public partial class Form1 : Form
{
    addDynamicCptboxComponents addcomponent = new addDynamicCptboxComponents();
    addDynamicSpellboxComponents addSpellboxComponent = new addDynamicSpellboxComponents();
    addButtonComponent addButtonComponents = new addButtonComponent();
    addDynamicProviderCptComponents addProvidercptComponents = new addDynamicProviderCptComponents();
    addCptRecommendedTxtBox addCptRecommendedTxtBoxcomponents = new addCptRecommendedTxtBox();
    addDynamiContributorFindingsComboboxComponents addContributorFindingComponents = new addDynamiContributorFindingsComboboxComponents();
    addDynamicDateofServiceComponents adddoscomponents = new addDynamicDateofServiceComponents();

    public Form1()
    {
        try
        {
            InitializeComponent();
        }
        catch (Exception ex)
        {
            //Write ex.Message to a file
            using (StreamWriter outfile = new StreamWriter(@".\error.txt"))
            {
                outfile.Write(ex.Message.ToString());
            }
        }
        initalizeUserdefinedComponents();

    }

    public void createDynamiccomponents()
    {
        int value;
        if (int.TryParse(txtboxvalLines.Text, out value))
        {
            int txtno = int.Parse(txtboxvalLines.Text);
            int pointX = 125;
            int pointY = 24;
            int spPointX = 240;
            int spPointY = 24;
            int txtboxRecommendedpointX = 640;
            int txtboxRecommendedpointY = 24;
            int btnPointX = 1170;
            int btnPointY = 24;
            int cmbPointX = 520;
            int cmbPointY = 24;
            int pncPointX = 920;
            int pncPointY = 24;
            int finalcmbPointX = 780;
            int finalcmbPointY = 24;
            int DOSPointX = 35;
            int DOSPointY = 24;

            for (int i = 0; i < txtno; i++)
            {
                customize_panel();
                addcomboboxcomponent.add_dynamic_components(i, cmbPointX, cmbPointY, this, i);
                addButtonComponents.add_dynamic_components(i, btnPointX, btnPointY, this, i);
                addcomponent.add_dynamic_components(i, pointX, pointY, this, i);
                addSpellboxComponent.add_dynamic_components(i, spPointX, spPointY, this, i);
                addProvidercptComponents.add_dynamic_components(i, pncPointX, pncPointY, this, i);
                addCptRecommendedTxtBoxcomponents.add_dynamic_components(i, txtboxRecommendedpointX, txtboxRecommendedpointY, this, i);
                addContributorFindingComponents.add_dynamic_components(i, finalcmbPointX, finalcmbPointY, this, i);
                adddoscomponents.add_dynamic_components(i, DOSPointX, DOSPointY, this, i);
                panel1.Show();
                spPointY += 50;
                btnPointY += 50;
                cmbPointY += 50;
                pointY += 50;
                pncPointY += 50;
                txtboxRecommendedpointY += 50;
                finalcmbPointY += 50;
                DOSPointY += 50;
            }

            btnAdd.Enabled = false;
            txtboxvalLines.Enabled = false;
            panel1.Focus();
        }
        else
        {
            MessageBox.Show("Please provide the count of total Claim lines");
            txtboxvalLines.Text = "";
        }
    }
}
  1. I have a feeling that I'm repeating the same thing by only changing the component i.e TextBox, ComboBox etc. Is there any way to refactor it.

Please feel free to add some suggestion so that i can improve in design patterns.

Edit : I have added all the 8 classes that have been mentioned

\$\endgroup\$
  • \$\begingroup\$ I have a feeling that I'm repeating the same thing by only changing the component i.e Textbox , combobox etc. Is there any way to refactor it. - This will be difficult to answer because you're saying I have nearly 7 classes which inherit the abstract class. and you've posted only three of them so there is no way anyone can see pattern there. Btw, question 2 is off-topic. Such questions are better asked on Software Engineering. \$\endgroup\$ – t3chb0t Dec 5 '18 at 7:38
  • \$\begingroup\$ @t3chb0t thanks for your comment. I will add other classes also. \$\endgroup\$ – Ramji R Dec 5 '18 at 7:40
  • \$\begingroup\$ Great! One more thing, is my assumption correct that the createDynamiccomponents method is a 1:1 copy and the only thing you've removed is the autogenerated InitializeComponent? \$\endgroup\$ – t3chb0t Dec 5 '18 at 11:04
4
\$\begingroup\$

The code is not entirely wrong but it's not appealing either. Here are a couple of thoughts:

  • In general I'd say this is ok and as far as WinForms are concerned not. Each of the abstract classes should be UserControl that you place in the specific location. This way it would be much easier to design them and I bet it's a pain to get all these indexes correctly.

  • You use magic-numbers a lot and many them are the same. Most of them should be constants, problably something like padding, margin, gap etc. It would be much easier to adjust them when they have meanings and do this for all occurances without having to search each one of them.

  • I must also say that your naming convention in great need of improvement. Current names are short an confusing. You use a lot of abbreviations that don't mean anything or you use a very unconventional class and method names. You should take a look at some C# Coding Standards and try to follow them as much as possible. If you make an exception then you should have a pretty good reason for doing so.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer, Can you please explain me more on the Each of the abstract classes should be UserControl that you place in the specific location. This way it would be much easier to design them and I bet it's a pain to get all these indexes correctly. \$\endgroup\$ – Ramji R Dec 6 '18 at 1:20
  • \$\begingroup\$ @RamjiR well, just look for UserControls in msdn or anywhere else, there are plenty of tutorials how this works. \$\endgroup\$ – t3chb0t Dec 19 '18 at 17:37
1
\$\begingroup\$

I don't know why you add the stuff to the form controls twice in most of the classes, so I am going to assume you don't need to do that.

I would suggest moving the x and y into the component classes, it will help with your for loop.

I took out tabIndex because you were passing i in for that and your number parameter, you can keep it in if you like it doesn't really change the underlying principles.

I am going to use generics to show how it would work with them, you can do it without generics but I don't want to do two examples, I use virtual for stuff that only some child classes will override and abstract for ones that every child class needs to override, because of the way collections work in c# I am having 2 levels of abstract classes, one that uses generics and one that doesn't

// abstract class
public abstract class add_components
{
    protected abstract int _offset { get; }
    protected abstract string _name { get; }
    protected virtual int _width => 75;
    protected virtual int _height => 23;
    public int x { get; set; }
    public int y { get; set; }
    public abstract void add_dynamic_components(int number, Form1 form)
}

public abstract class add_components<TControl> : add_components
    where TControl : Control, new()
{
    public override void add_dynamic_components(int number, Form1 form)
        var control = new TControl();
        form.panel1.Controls.Add(control);
        control.Location = new Point(x, y);
        control.Size = new System.Drawing.Size(_width, _height);
        control.Name = "Add_" + _name + number;
        control.TabIndex = number * 7 + _offset;
        DoControlSpecific(control);
    }

    protected virtual void DoControlSpecific(TControl control)
    {
    }
}

public class addButtonComponents : add_components<Button>
{
    protected override int _offset => 8;
    protected override string _name => "Button";

    protected override void DoControlSpecific(Button control)
    {
        control.Text = "Add";
        control.Click += new EventHandler(f.btnAddCptInformation);
    }
}

// inherit from add_components<TControl> for your other controls

using System.Collections.Generics;

public partial class Form1 : Form
{
    addButtonComponent addButtonComponents = new addButtonComponent();

    public void createDynamiccomponents()
    {
        int value;
        if (int.TryParse(txtboxvalLines.Text, out value))
        {
            List<add_components> addComponents = new List<add_components>
            {
                addButtonComponents
                // all other components go in this list
            };
            addButtonComponents.x = 1170;
            addButtonComponents.y = 24;
            // set all other components x and y here.

            for (int i = 0; i < txtno; i++)
            {
                customize_panel();
                foreach (add_component addComponent in addComponents)
                {
                    addComponent.add_dynamic_components(i, this, i);
                    addComponent.y += 50;
                }
                panel1.Show();
            }

            btnAdd.Enabled = false;
            txtboxvalLines.Enabled = false;
            panel1.Focus();
        }
        else
        {
            MessageBox.Show("Please provide the count of total Claim lines");
            txtboxvalLines.Text = "";
        }
    }
}

This code may have syntax errors or bugs I didn't write it with a ide.

The abstract class type can be given to your generic list so that you can loop through that list and call the same method on each

  • you could accomplish that with an interface

but you can't define protected properties or methods in an interface

  • you could accomplish that with a non abstract base class

but you can't define abstract methods in a non abstract base class which means you can't force your child classes to override things

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.