-1

I have a working code that has to many lines of repeated code. I would like to write a method to save lines of code.

I have a query that gets the number of registries in the database. The number of registries may vary from 1 to 20. The number of registries is saved in the decimal numberOfLines.

Today I am using 20 if's, that writes 4 textboxes per line for each possible number of lines I have.

If I have only one line it write 1 line of 4 textboxes, if I have 2 lines it writes 2 lines of 4 textboxes and so one.

I will only show the code for the quantity of lines from 1 to 4 just to save space (the rest of the code is just copy/paste and change the textboxes new indexes).

//write the lines according to the number of lines

        if (numberOfLines == 1)
        {
            txt_A1.Text = tableAs.Rows[0][0].ToString();
            txt_B1.Text = tableAs.Rows[0][1].ToString();
            txt_C1.Text = tableAs.Rows[0][2].ToString();
            txt_D1.Text = tableAs.Rows[0][3].ToString();
        }
        if (numberOfLines ==2)
        {
            txt_A1.Text = tableAs.Rows[0][0].ToString();
            txt_B1.Text = tableAs.Rows[0][1].ToString();
            txt_C1.Text = tableAs.Rows[0][2].ToString();
            txt_D1.Text = tableAs.Rows[0][3].ToString();
            txt_A2.Text = tableAs.Rows[1][0].ToString();
            txt_B2.Text = tableAs.Rows[1][1].ToString();
            txt_C2.Text = tableAs.Rows[1][2].ToString();
            txt_D2.Text = tableAs.Rows[1][3].ToString();
        }
        if (numberOfLines == 3)
        {
            txt_A1.Text = tableAs.Rows[0][0].ToString();
            txt_B1.Text = tableAs.Rows[0][1].ToString();
            txt_C1.Text = tableAs.Rows[0][2].ToString();
            txt_D1.Text = tableAs.Rows[0][3].ToString();
            txt_A2.Text = tableAs.Rows[1][0].ToString();
            txt_B2.Text = tableAs.Rows[1][1].ToString();
            txt_C2.Text = tableAs.Rows[1][2].ToString();
            txt_D2.Text = tableAs.Rows[1][3].ToString();
            txt_A3.Text = tableAs.Rows[2][0].ToString();
            txt_B3.Text = tableAs.Rows[2][1].ToString();
            txt_C3.Text = tableAs.Rows[2][2].ToString();
            txt_D3.Text = tableAs.Rows[2][3].ToString();
        }
        if (numberOfLines == 4)
        {
            txt_A1.Text = tableAs.Rows[0][0].ToString();
            txt_B1.Text = tableAs.Rows[0][1].ToString();
            txt_C1.Text = tableAs.Rows[0][2].ToString();
            txt_D1.Text = tableAs.Rows[0][3].ToString();
            txt_A2.Text = tableAs.Rows[1][0].ToString();
            txt_B2.Text = tableAs.Rows[1][1].ToString();
            txt_C2.Text = tableAs.Rows[1][2].ToString();
            txt_D2.Text = tableAs.Rows[1][3].ToString();
            txt_A3.Text = tableAs.Rows[2][0].ToString();
            txt_B3.Text = tableAs.Rows[2][1].ToString();
            txt_C3.Text = tableAs.Rows[2][2].ToString();
            txt_D3.Text = tableAs.Rows[2][3].ToString();
            txt_A4.Text = tableAs.Rows[3][0].ToString();
            txt_B4.Text = tableAs.Rows[3][1].ToString();
            txt_C4.Text = tableAs.Rows[3][2].ToString();
            txt_D4.Text = tableAs.Rows[3][3].ToString();
        }

With the amount of possible lines I have (20) the code gets very big, and not as beautiful as I expected (with the use of a method for example).

Marcello Silva
  • 152
  • 2
  • 14
  • You could store your textboxes in an array and access them by index so that way you could use a simple loop(s) – maccettura Sep 30 '19 at 16:26
  • Why don't use [for statement](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/for)? Or maybe a [foreach statement](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/foreach-in)? – Antonio Veneroso Contreras Sep 30 '19 at 16:33
  • There are lots of different ways one might improve the code. There's not enough context to know if there is _one specific_ way that you need here, making your question far too broad. That said, I'll point out that if you were using WPF, this would not even be a question, because you'd be able to declare an "items control" that automatically populates itself with the appropriate number of text box groups, and automatically binds those text boxes to the data via a view model that understands the database access. FWIW, Winforms can accomplish much the same, albeit with less convenience. – Peter Duniho Sep 30 '19 at 21:25
  • @AntonioVenerosoContreras I don't know how simply using "for" or "foreach" could save lines of code. Can you give an example? – Marcello Silva Oct 01 '19 at 12:16
  • Can you show your data? How many tables are involved? I don't fully understand your question and I believe that's the reason. – Antonio Veneroso Contreras Oct 01 '19 at 22:55
  • @AntonioVenerosoContreras Sure. It's only one table in the DB, it has a reference value (int not null), and A, B, C and D values (string). For each reference, the DB can have from 1 up to 20 registries. The form has 20 rows of 4 columns (each row corresponding to a set of A, B, C and D). I have to fill the form with DB data corresponding to a given ref number. If I have only 1 registry for a given ref number, the form shows this single row. If I have 2 registries, it shows 2 rows, and so on. let me know if more clarification is needed. – Marcello Silva Oct 02 '19 at 12:49
  • o_O I think I understand now: you have 80 textboxes in the form? Perhaps [this post](https://stackoverflow.com/questions/3419159/how-to-get-all-child-controls-of-a-windows-forms-form-of-a-specific-type-button) helps you understand how you could iterate all of them assigning the corresponding value. You would need to name your textboxes in a friendly way it can be checked in the foreach loop. Good luck! – Antonio Veneroso Contreras Oct 02 '19 at 13:46
  • @AntonioVenerosoContreras Yes, I have 80 textboxes, 20 rows x 4 columns. I will take a look at the post, thanks. The main problem I have now is I can't read a value that doesn't exist in the DB so I think I have to use try/catch block to avoid repeated code, like, I try to read all 20 lines regardless the number of lines . – Marcello Silva Oct 02 '19 at 15:23

2 Answers2

0

Try this refactoring:

var items = new Dictionary<int, List<TextBox>>()
{
  { 1, new List<TextBox>() { txt_A1, txt_B1, txt_C1, txt_D1 } },
  { 2, new List<TextBox>() { txt_A1, txt_B1, txt_C1, txt_D1, txt_A2, txt_B2, txt_C2, txt_D2 } },
  { 3, new List<TextBox>() { txt_A1, txt_B1, txt_C1, txt_D1, txt_A2, txt_B2, txt_C2, txt_D2, txt_A3, txt_B3, txt_C3, txt_D3 } },
  { 4, new List<TextBox>() { txt_A1, txt_B1, txt_C1, txt_D1, txt_A2, txt_B2, txt_C2, txt_D2, txt_A3, txt_B3, txt_C3, txt_D3, txt_A4, txt_B4, txt_C4, txt_D4 } }
};

int index1 = 0;
int index2 = 0;
foreach ( var item in items[numberOfLines] )
{
  item.Text = tableAs.Rows[index1][index2].ToString();
  if ( ++index2 > 3 )
  {
    index2 = 0;
    index1++;
  }
}
  • As I understand, the problem is the OP has 20 rows, so I believe you need to refactor your dictionary for the 20 rows... that's why I suggested a for or foreach loop – Antonio Veneroso Contreras Sep 30 '19 at 17:16
  • I've tried. The form has indeed 20 lines (with 4 textboxes each) but the database (as I've mentioned in the description) may vary from 1 to 20 entries. I tested your code, it fails when the number of entries in the database is one. I get: [IndexOutOfRangeException: Aucune ligne à la position 1.] – Marcello Silva Oct 01 '19 at 12:51
  • I made a copy/paste mistake about the dictionary keys (start from 1, not 0... because of numberOfLines is not base index from 0...). Answer updated. I don't know if your error is related to this. –  Oct 01 '19 at 13:07
0

I write a method that you could create textboxes according to the line. Code:

 private void button1_Click(object sender, EventArgs e)
        {
            int a = Convert.ToInt32(richTextBox1.Text);
            CreateTextbox(a);
        }

        public void CreateTextbox(int line)
        {
            int count = 0;
            int num = 0;
            for (int i = 0; i < line*4; i++)
            {
                TextBox box = new TextBox();
                box.Name = "A" + i.ToString();
                if (count >= 4)
                {

                    count = 0;
                    num++;
                }

                box.Location = new Point(count*(box.Width+20),num*40);
                count++;
                this.Controls.Add(box);
            }


        }
    }

Result Picture

Jack J Jun
  • 5,633
  • 1
  • 9
  • 27