Dynamically adding controls to a form in a WinForms project

Question

I have created this function in my DetailsScreen.cs, but I have no idea if this is the correct file to put this kind of code in, or if coding it this way is the correct approach. I know that it works because I have tested it.

My application generates a questionnaire based on selected (cyber security) standards. I have a form DetailsScreen, which requires the user to fill in some details such as name, company, etc. They also need to select from a checkedlist (questionStandardInput), the question standard they wish to use in this interview. For each selected standard, two answer standards need to be selected from two drop down lists (maturity answer standard and compliance answers standard).

Therefore, I need to be able to dynamically add labels and comboxes for each question standard. I asked because most code concerning label font etc. is placed in .Designer.cs files instead of the .cs files. But I need certain logic from the .cs file in order to properly format the controls, which is why I put it in here.

// Load answer input according to the number of questions standards selected.
private void questionStandardInput_MouseUp(object sender, MouseEventArgs e)
{
    questionLabels.Clear();
    maturityInput.Clear();
    complianceInput.Clear();
    selectionPanel.Controls.Clear();

    foreach (var item in questionStandardInput.CheckedItems)
    {
        Label qs_label = new Label();
        qs_label.Location = new Point(0, 0 + (questionLabels.Count * 115));
        qs_label.Font = new Font("Arial", 12F, FontStyle.Regular);
        qs_label.Size = new Size(150, 18);
        qs_label.Text = item.ToString();
        questionLabels.Add(qs_label);
        this.selectionPanel.Controls.Add(qs_label);

        Label m_label = new Label();
        m_label.Location = new Point(0, 0 + (questionLabels.Count * 115 + 35));
        m_label.Font = new Font("Arial", 12F, FontStyle.Regular);
        m_label.Size = new Size(150, 18);
        m_label.Text = "Maturity standard";
        this.selectionPanel.Controls.Add(m_label);

        Label c_label = new Label();
        c_label.Location = new Point(0, 0 + (questionLabels.Count * 115 + 70));
        c_label.Font = new Font("Arial", 12F, FontStyle.Regular);
        c_label.Size = new Size(170, 18);
        c_label.Text = "Compliance standard";
        this.selectionPanel.Controls.Add(c_label);

        ComboBox m_input = new ComboBox();
        m_input.Location = new Point(170, 0 + (questionLabels.Count * 115 + 35));
        m_input.Font = new Font("Arial", 12F, FontStyle.Regular);
        m_input.Size = new Size(200, 22);
        m_input.DropDownStyle = ComboBoxStyle.DropDownList;
        maturityInput.Add(m_input);
        this.selectionPanel.Controls.Add(m_input);

        ComboBox c_input = new ComboBox();
        c_input.Location = new Point(170, 0 + (questionLabels.Count * 115 + 70));
        c_input.Font = new Font("Arial", 12F, FontStyle.Regular);
        c_input.Size = new Size(200, 22);
        c_input.DropDownStyle = ComboBoxStyle.DropDownList;
        complianceInput.Add(c_input);
        this.selectionPanel.Controls.Add(c_input);

        InitializeMaturityStandardInput(m_input);
        InitializeComplianceStandardInput(c_input);
    }
    this.saveAssessmentButton.Location = new Point(235, 0 + (questionLabels.Count * 115));
    this.selectionPanel.Controls.Add(saveAssessmentButton); 
}

Answer 1

In my eyes it is an appropriated way to build up your form that way. The *.Designer file is for designer generated code - but if you need to add custom logic to modify your GUI the *.cs file is the correct location.

Just a few points:

• There are some numbers (115, 70, 35, ...) that seems to have a special meening (e.g. row height, margin, whatever). IMHO it makes sense to store that numbers as constants with a meaningful name and use that constants instead.

• I am not sure if the MouseUp event is the right place to trigger that kind of logic. If there is another event (CheckBoxChanged, SelectedItemChanged, ...) I would suggest to use that.

• There is some repeating logic for creating the ComboBox / Label and so on. That logic could be extracted to a separted method. For instance:

  private static ComboBox CreateComboBox()
  {
      return new ComboBox()
      {
          Font = new Font("Arial", 12F, FontStyle.Regular)
          Size = new Size(200, 22)
          DropDownStyle = ComboBoxStyle.DropDownList
      }
  }
  

Answer 2

Adding to @JanDotNet's answer I think you should create a UserControl that knows how to generate those items.

It is a very bad idea to put business logic inside an event handler (questionStandardInput_MouseUp).

If you had a specialized user control you could then very easy (re)place one user control with another according to the user choice and you wouldn't have to care about so many coordinates. They would be relative to the user control and in its context always correct.

Would you change the form layout now you would probably need to adjust all locations.

Answer 3

I have come up with my own solution with your help and some research. I have implemented the Model View Controller Passive View so that no logic is handled in the View. I still have a few loose digits to clean up, but that was not the main focus.

In the View's MouseUp event handler just two events are fired. One for the Presenter that will get data from the Model (that gets it from the database) which is passed to the User Control through the View, and one for the User Control to initialize itsself. The User Control is then added to the View.

View MouseUp event handler

// Fires the initializing user control and initializing user control data events.
private void questionStandardInput_MouseUp(object sender, MouseEventArgs e)
{
    InitializingUserControl?.Invoke(this, EventArgs.Empty);
    InitializingUserControlData?.Invoke(this, EventArgs.Empty);
}

User Control constructor and Initializing function

    // Initialize user control with IDetailScreenView. Subscribe to necessary events.
    public DetailScreenUserControl(IDetailScreenView view)
    {
        InitializeComponent();
        _view = view;
        _view.InitializingUserControl += InitializeUserControl;
    }

    // Initializes the user control for the detail screen.
    public void InitializeUserControl(object sender, EventArgs e)
    {
        List<string> qStandards = _view.SelectedQuestionStandards;
        Controls.Clear();
        maturityInput.Clear();
        complianceInput.Clear();

        int inputSeparation = 115;
        int secondInputX = 35;
        int inputHeight = 20;
        int labelWidth = 200;
        int inputWidth = 170;

        for (int  i = 0; i < qStandards.Count; i++)
        {
            Panel inputPanel = new Panel();
            inputPanel.Location = new Point(0, i * inputSeparation);
            inputPanel.AutoSize = true;
            inputPanel.BackColor = Color.AliceBlue;
            this.Controls.Add(inputPanel);

            Label qs_label = new Label();
            qs_label.Location = new Point(0, 0);
            qs_label.Font = new Font("Arial", 12F, FontStyle.Bold);
            qs_label.Size = new Size(labelWidth, inputHeight);
            qs_label.AutoSize = true;
            qs_label.Text = qStandards[i].ToString();
            inputPanel.Controls.Add(qs_label);

            Label m_label = new Label();
            m_label.Location = new Point(0, secondInputX);
            m_label.Font = new Font("Arial", 12F, FontStyle.Regular);
            m_label.Size = new Size(labelWidth, inputHeight);
            m_label.Text = "Maturity standard";
            inputPanel.Controls.Add(m_label);

            Label c_label = new Label();
            c_label.Location = new Point(0, secondInputX * 2);
            c_label.Font = new Font("Arial", 12F, FontStyle.Regular);
            c_label.Size = new Size(labelWidth, inputHeight);
            c_label.Text = "Compliance standard";
            inputPanel.Controls.Add(c_label);

            ComboBox m_input = new ComboBox();
            m_input.Location = new Point(labelWidth, secondInputX);
            m_input.Font = new Font("Arial", 10F, FontStyle.Regular);
            m_input.Size = new Size(inputWidth, inputHeight);
            m_input.DropDownStyle = ComboBoxStyle.DropDownList;
            maturityInput.Add(m_input);
            inputPanel.Controls.Add(m_input);

            ComboBox c_input = new ComboBox();
            c_input.Location = new Point(labelWidth, secondInputX * 2);
            c_input.Font = new Font("Arial", 10F, FontStyle.Regular);
            c_input.Size = new Size(inputWidth, inputHeight);
            c_input.DropDownStyle = ComboBoxStyle.DropDownList;
            complianceInput.Add(c_input);
            inputPanel.Controls.Add(c_input);

        }

        saveAssessmentButton.Location = new Point(195, qStandards.Count * inputSeparation);
        saveAssessmentButton.Size = new Size(130, 25);
        saveAssessmentButton.Text = "Save assessment";
        saveAssessmentButton.BackColor = SystemColors.ButtonHighlight;
        this.Controls.Add(saveAssessmentButton);

        _view.AddUserControl();
    }