1
\$\begingroup\$

my winform app contains a combo box, a button and two datagridview among other controls. Both DGV's have 3 columns. When I select an item/items from one or both of the DGV's and hit the button the app would create/open a word file(*.docx) and first add a header and a date on the top,then add a table with the selected DGV rows with a sum of the 3rd DGV column in a new page.

Example 1 (only 1st DGV items are selected)

dgv1

The resulting word file should look like this ![docx

Example 2 (both DGV items are selected)

both dgv

Then the resulting word file should look like enter image description here

The code I've done, the rough idea behind it is like this (its messy, badly organized and redundant and needs refinement):

  1. Check if any row/rows from the 2nd DGV is chosen or not, if yes, you cannot select more than 3 in the 2nd DGV & also you cannot select more than 8 rows in the 1st DGV when any item from the 2nd DGV is chosen.

  2. If the word file exists open it or create a new file

  3. Calculate the sum of the 3 rd columns of the selected rows of both the DGV's and Calculate their difference (DGV1-DGV2)

  4. Add the selected rows in the word file in table format (see image below)

  5. If no row/rows from the 2nd DGV is chosen, Check how many rows from the 1st DGV are selected. Minimum 1 item and/or maximum 15 items can be chosen. Then follow the steps from 2-4, only calculate sum of the 3rd columns of selected rows of DGV1 in this case.

         string fileName=@"G:\forwarding.docx";
    
    
         if (kryptonDataGridView2.SelectedRows.Count > 0)
         {
             if (kryptonDataGridView2.SelectedRows.Count > 3)
             {
                 MessageBox.Show(this,"Please select 3 or less Credit/Debit notes","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
                 return;
             }
    
    
             if (kryptonDataGridView1.SelectedRows.Count > 0 && kryptonDataGridView1.SelectedRows.Count > 8)
    
             {
                 MessageBox.Show(this,"Please select atleast 1 bill and/or maximum 8 bills","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
             }
    
             else
             {
                 decimal total = kryptonDataGridView1.SelectedRows.OfType<DataGridViewRow>()
                     .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));
                 decimal _total = kryptonDataGridView2.SelectedRows.OfType<DataGridViewRow>()
                     .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));
    
                 if (_total > total)
                 {
                     MessageBox.Show(this,"Credit/Debit note total value cannot be higher than total bill value","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
                     return;
                 }
    
                 decimal adj_total=total - _total;
    
    
                 if (File.Exists(fileName))
                 {
    
                     using( var document = DocX.Load( fileName ) )
                     {
    
                         document.AddHeaders();
    
                         Xceed.Document.NET.Paragraph para=document.InsertParagraph();
                         para.InsertPageBreakBeforeSelf();
    
                         Xceed.Document.NET.Header header_default = document.Headers.Odd;
                         Xceed.Document.NET.Paragraph p1 = header_default.InsertParagraph();
                         p1.FontSize(20).Append("W").Bold().Font("Arial").Color(Color.Red);
                         p1.FontSize(20).Append("I").Bold().Font("Arial").Color(Color.Brown);
                         p1.FontSize(20).Append("Z").Bold().Font("Arial").Color(Color.Gold);
                         p1.FontSize(20).Append("D").Bold().Font("Arial").Color(Color.Chocolate);
                         p1.FontSize(20).Append("E").Bold().Font("Arial").Color(Color.Teal);
                         p1.FontSize(20).Append("R").Bold().Font("Arial").Color(Color.Violet);
                         p1.FontSize(20).Append("M").Bold().Font("Arial").Color(Color.Orange);
    
    
                         p1.Alignment=Alignment.center;
                         p1.FontSize(20);
    
                         Xceed.Document.NET.Paragraph fdate=document.InsertParagraph();
                         fdate.Append("                                       Date:  "+DateTime.Today.ToString("dd-MMM-yyyy"));
                         fdate.Alignment=Alignment.right;
    
                         Xceed.Document.NET.Paragraph blank=document.InsertParagraph();
    
                         var tab = document.AddTable( 1, 3 );
                         tab.Design = TableDesign.TableGrid;
                         tab.Alignment=Alignment.center;
                         tab.Rows[0].MergeCells(0,2);
                         tab.AutoFit=AutoFit.Contents;
                         tab.Rows[0].Cells[0].Paragraphs.First().Append(kryptonComboBox1.Text).Bold().Alignment=Alignment.center;
    
                         document.InsertTable( tab );
    
    
                         var table = document.AddTable( kryptonDataGridView1.SelectedRows.Count+3, kryptonDataGridView1.Columns.Count );
                         table.Design = TableDesign.TableGrid;
                         table.Alignment=Alignment.center;
                         table.AutoFit = AutoFit.Contents;
    
                         int rowNumber = 0;
    
                         int columnNumber = 0;
                         foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                         {
                             table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
                             columnNumber++;
                         }
    
                         rowNumber++;
    
                         foreach (DataGridViewRow row in kryptonDataGridView1.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
                         {
                             columnNumber = 0;
                             foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                             {
                                 table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(row.Cells[columnNumber].Value.ToString());
                                 columnNumber++;
                             }
    
                             rowNumber++;
                         }
    
    
    
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(total.ToString()).Bold(true);
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].MergeCells(0,1);
    
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+2].Cells[0].Paragraphs.First().Append("Credit/Debit Note").Bold(true).Alignment=Alignment.center;
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+2].MergeCells(0,2);
    
                         document.InsertTable(table);
    
                         var cndn_table = document.AddTable( kryptonDataGridView2.SelectedRows.Count+3, kryptonDataGridView2.Columns.Count );
                         cndn_table.Design = TableDesign.TableGrid;
                         cndn_table.Alignment=Alignment.center;
                         cndn_table.AutoFit = AutoFit.Contents;
    
                         int _rowNumber = 0;
    
                         int _columnNumber = 0;
                         foreach (DataGridViewColumn column in kryptonDataGridView2.Columns)
                         {
                             cndn_table.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
                             _columnNumber++;
                         }
    
                         _rowNumber++;
    
                         foreach (DataGridViewRow row in kryptonDataGridView2.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
                         {
                             _columnNumber = 0;
                             foreach (DataGridViewColumn column in kryptonDataGridView2.Columns)
                             {
                                 cndn_table.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(row.Cells[_columnNumber].Value.ToString());
                                 _columnNumber++;
                             }
    
                             _rowNumber++;
                         }
    
    
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(_total.ToString()).Bold(true);
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+1].MergeCells(0,1);
    
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+2].Cells[0].Paragraphs.First().Append("Total after adjustment :").Bold(true).Alignment=Alignment.right;
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+2].Cells[2].Paragraphs.First().Append(adj_total.ToString()).Bold(true);
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+2].MergeCells(0,1);
    
                         document.InsertTable(cndn_table);
    
    
                         document.Save();
                     }
                 }
                 else
                 {
                     using( var document = DocX.Create( fileName ) )
                     {
    
                         document.AddHeaders();
    
                         Xceed.Document.NET.Paragraph para=document.InsertParagraph();
    
                         Xceed.Document.NET.Header header_default = document.Headers.Odd;
                         Xceed.Document.NET.Paragraph p1 = header_default.InsertParagraph();
                         p1.FontSize(20).Append("W").Bold().Font("Arial").Color(Color.Red);
                         p1.FontSize(20).Append("I").Bold().Font("Arial").Color(Color.Brown);
                         p1.FontSize(20).Append("Z").Bold().Font("Arial").Color(Color.Gold);
                         p1.FontSize(20).Append("D").Bold().Font("Arial").Color(Color.Chocolate);
                         p1.FontSize(20).Append("E").Bold().Font("Arial").Color(Color.Teal);
                         p1.FontSize(20).Append("R").Bold().Font("Arial").Color(Color.Violet);
                         p1.FontSize(20).Append("M").Bold().Font("Arial").Color(Color.Orange);
    
    
                         p1.Alignment=Alignment.center;
                         p1.FontSize(20);
    
                         Xceed.Document.NET.Paragraph fdate=document.InsertParagraph();
                         fdate.Append("                                       Date:  "+DateTime.Today.ToString("dd-MMM-yyyy"));
                         fdate.Alignment=Alignment.right;
    
                         Xceed.Document.NET.Paragraph blank=document.InsertParagraph();
    
                         var tab = document.AddTable( 1, 3 );
                         tab.Design = TableDesign.TableGrid;
                         tab.Alignment=Alignment.center;
                         tab.Rows[0].MergeCells(0,2);
                         tab.AutoFit=AutoFit.Contents;
                         tab.Rows[0].Cells[0].Paragraphs.First().Append(kryptonComboBox1.Text).Bold().Alignment=Alignment.center;
    
                         document.InsertTable( tab );
    
    
                         var table = document.AddTable( kryptonDataGridView1.SelectedRows.Count+3, kryptonDataGridView1.Columns.Count );
                         table.Design = TableDesign.TableGrid;
                         table.Alignment=Alignment.center;
                         table.AutoFit = AutoFit.Contents;
    
                         int rowNumber = 0;
    
                         int columnNumber = 0;
                         foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                         {
                             table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
                             columnNumber++;
                         }
    
                         rowNumber++;
    
                         foreach (DataGridViewRow row in kryptonDataGridView1.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
                         {
                             columnNumber = 0;
                             foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                             {
                                 table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(row.Cells[columnNumber].Value.ToString());
                                 columnNumber++;
                             }
    
                             rowNumber++;
                         }
    
    
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(total.ToString()).Bold(true);
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].MergeCells(0,1);
    
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+2].Cells[0].Paragraphs.First().Append("Credit/Debit Note").Bold(true).Alignment=Alignment.center;
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+2].MergeCells(0,2);
    
                         document.InsertTable(table);
    
                         var cndn_table = document.AddTable( kryptonDataGridView2.SelectedRows.Count+3, kryptonDataGridView2.Columns.Count );
                         cndn_table.Design = TableDesign.TableGrid;
                         cndn_table.Alignment=Alignment.center;
                         cndn_table.AutoFit = AutoFit.Contents;
    
                         int _rowNumber = 0;
    
                         int _columnNumber = 0;
                         foreach (DataGridViewColumn column in kryptonDataGridView2.Columns)
                         {
                             cndn_table.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
                             _columnNumber++;
                         }
    
                         _rowNumber++;
    
                         foreach (DataGridViewRow row in kryptonDataGridView2.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
                         {
                             _columnNumber = 0;
                             foreach (DataGridViewColumn column in kryptonDataGridView2.Columns)
                             {
                                 cndn_table.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(row.Cells[_columnNumber].Value.ToString());
                                 _columnNumber++;
                             }
    
                             _rowNumber++;
                         }
    
    
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(_total.ToString()).Bold(true);
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+1].MergeCells(0,1);
    
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+2].Cells[0].Paragraphs.First().Append("Total after adjustment :").Bold(true).Alignment=Alignment.right;
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+2].Cells[2].Paragraphs.First().Append(adj_total.ToString()).Bold(true);
                         cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count+2].MergeCells(0,1);
    
                         document.InsertTable(cndn_table);
    
    
                         document.Save();
                     }
                 }
    
                 MessageBox.Show(this,"Done","Info",MessageBoxButtons.OK,MessageBoxIcon.Information);
                 kryptonDataGridView1.Rows.Clear();
                 kryptonDataGridView2.Rows.Clear();
             }
         }
         else
         {
             if (kryptonDataGridView1.SelectedRows.Count < 1 || kryptonDataGridView1.SelectedRows.Count > 15)
             {
                 MessageBox.Show(this,"Please select minimum 1 item and/or maximum 15 items","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
             }
             else
             {
                 if (File.Exists(fileName))
                 {
    
                     using( var document = DocX.Load( fileName ) )
                     {
    
                         document.AddHeaders();
    
                         Xceed.Document.NET.Paragraph para=document.InsertParagraph();
                         para.InsertPageBreakBeforeSelf();
    
                         Xceed.Document.NET.Header header_default = document.Headers.Odd;
                         Xceed.Document.NET.Paragraph p1 = header_default.InsertParagraph();
                         p1.FontSize(20).Append("W").Bold().Font("Arial").Color(Color.Red);
                         p1.FontSize(20).Append("I").Bold().Font("Arial").Color(Color.Brown);
                         p1.FontSize(20).Append("Z").Bold().Font("Arial").Color(Color.Gold);
                         p1.FontSize(20).Append("D").Bold().Font("Arial").Color(Color.Chocolate);
                         p1.FontSize(20).Append("E").Bold().Font("Arial").Color(Color.Teal);
                         p1.FontSize(20).Append("R").Bold().Font("Arial").Color(Color.Violet);
                         p1.FontSize(20).Append("M").Bold().Font("Arial").Color(Color.Orange);
    
    
                         p1.Alignment=Alignment.center;
                         p1.FontSize(20);
    
                         Xceed.Document.NET.Paragraph fdate=document.InsertParagraph();
                         fdate.Append("                                       Date:  "+DateTime.Today.ToString("dd-MMM-yyyy"));
                         fdate.Alignment=Alignment.right;
    
                         Xceed.Document.NET.Paragraph blank=document.InsertParagraph();
    
                         var tab = document.AddTable( 1, 3 );
                         tab.Design = TableDesign.TableGrid;
                         tab.Alignment=Alignment.center;
                         tab.Rows[0].MergeCells(0,2);
                         tab.AutoFit=AutoFit.Contents;
                         tab.Rows[0].Cells[0].Paragraphs.First().Append(kryptonComboBox1.Text).Bold().Alignment=Alignment.center;
    
                         document.InsertTable( tab );
    
    
                         var table = document.AddTable( kryptonDataGridView1.SelectedRows.Count+2, kryptonDataGridView1.Columns.Count );
                         table.Design = TableDesign.TableGrid;
                         table.Alignment=Alignment.center;
                         table.AutoFit = AutoFit.Contents;
    
                         int rowNumber = 0;
    
                         int columnNumber = 0;
                         foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                         {
                             table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
                             columnNumber++;
                         }
    
                         rowNumber++;
    
                         foreach (DataGridViewRow row in kryptonDataGridView1.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
                         {
                             columnNumber = 0;
                             foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                             {
                                 table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(row.Cells[columnNumber].Value.ToString());
                                 columnNumber++;
                             }
    
                             rowNumber++;
                         }
    
                         decimal total = kryptonDataGridView1.SelectedRows.OfType<DataGridViewRow>()
                             .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));
    
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].Cells[0].Paragraphs.First().Append("Total :").Bold(true).Alignment=Alignment.right;
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(total.ToString()).Bold(true);
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].MergeCells(0,1);
    
                         document.InsertTable(table);
    
                         document.Save();
                     }
                 }
                 else
                 {
                     using( var document = DocX.Create( fileName ) )
                     {
    
                         document.AddHeaders();
    
                         Xceed.Document.NET.Paragraph para=document.InsertParagraph();
    
                         Xceed.Document.NET.Header header_default = document.Headers.Odd;
                         Xceed.Document.NET.Paragraph p1 = header_default.InsertParagraph();
                         p1.FontSize(20).Append("W").Bold().Font("Arial").Color(Color.Red);
                         p1.FontSize(20).Append("I").Bold().Font("Arial").Color(Color.Brown);
                         p1.FontSize(20).Append("Z").Bold().Font("Arial").Color(Color.Gold);
                         p1.FontSize(20).Append("D").Bold().Font("Arial").Color(Color.Chocolate);
                         p1.FontSize(20).Append("E").Bold().Font("Arial").Color(Color.Teal);
                         p1.FontSize(20).Append("R").Bold().Font("Arial").Color(Color.Violet);
                         p1.FontSize(20).Append("M").Bold().Font("Arial").Color(Color.Orange);
    
    
                         p1.Alignment=Alignment.center;
                         p1.FontSize(20);
    
                         Xceed.Document.NET.Paragraph fdate=document.InsertParagraph();
                         fdate.Append("                                       Date:  "+DateTime.Today.ToString("dd-MMM-yyyy"));
                         fdate.Alignment=Alignment.right;
    
                         Xceed.Document.NET.Paragraph blank=document.InsertParagraph();
    
                         var tab = document.AddTable( 1, 3 );
                         tab.Design = TableDesign.TableGrid;
                         tab.Alignment=Alignment.center;
                         tab.Rows[0].MergeCells(0,2);
                         tab.AutoFit=AutoFit.Contents;
                         tab.Rows[0].Cells[0].Paragraphs.First().Append(kryptonComboBox1.Text).Bold().Alignment=Alignment.center;
    
                         document.InsertTable( tab );
    
                         var table = document.AddTable( kryptonDataGridView1.SelectedRows.Count+2, kryptonDataGridView1.Columns.Count );
                         table.Design = TableDesign.TableGrid;
                         table.Alignment=Alignment.center;
                         table.AutoFit = AutoFit.Contents;
    
                         int rowNumber = 0;
    
                         int columnNumber = 0;
                         foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                         {
                             table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
                             columnNumber++;
                         }
    
                         rowNumber++;
    
                         foreach (DataGridViewRow row in kryptonDataGridView1.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
                         {
                             columnNumber = 0;
                             foreach (DataGridViewColumn column in kryptonDataGridView1.Columns)
                             {
                                 table.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(row.Cells[columnNumber].Value.ToString());
                                 columnNumber++;
                             }
    
                             rowNumber++;
                         }
    
                         decimal total = kryptonDataGridView1.SelectedRows.OfType<DataGridViewRow>()
                             .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));
    
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].Cells[0].Paragraphs.First().Append("Total :").Bold(true).Alignment=Alignment.right;
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(total.ToString()).Bold(true);
                         table.Rows[kryptonDataGridView1.SelectedRows.Count+1].MergeCells(0,1);
    
                         document.InsertTable(table);
    
                         document.Save();
                     }
                 }
    
                 MessageBox.Show(this,"Done","Info",MessageBoxButtons.OK,MessageBoxIcon.Information);
             }
         }
    
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. In this case the edit invalidated 2 answers. \$\endgroup\$
    – pacmaninbw
    Commented Jan 17, 2022 at 1:53
  • 1
    \$\begingroup\$ If you want to get a review of your new code please ask a follow up question that links back to this question. \$\endgroup\$
    – pacmaninbw
    Commented Jan 17, 2022 at 1:55

3 Answers 3

5
\$\begingroup\$

Review

I have gone through all the code, this is a part of a single function that seems to be handling an event in the user interface, possibly a selection event on a data grid.

There are 450 lines of code and that is way too much for a single function. A general rule in current programming is that any function larger than one screen in an editor is too large and needs to be broken down into smaller functions because it is too hard to follow the logic in the function. Visual Studio is kind enough to allow you to hide portions of the code, but I feel this is a bug in Visual Studio rather than a feature.

Programmers that are just starting out using Visual Studio, C# and Windows Forms quite often make the mistake of tying the display together with the data and the logic. The problem with this is that it creates monolithic blocks of code that are impossible to debug and maintain. I've had to professionally fix bugs in similar code that was in production. Generally I broke the code into smaller functions.

Move the data into a separate module (C# class) that performs all necessary logic. You can bind a DataTable from System.Data to a DataGridView. Perform all necessary actions on the DataTable rather than on the DataGridView. The DataGridView should be purely for presenting tabular data in the user interface, I have found it difficult to perform calculations and modifications to the data when trying to manipulate the data in the DataGridView.

Move the code to generate the word document into another C# class that is only concerned with generating the word document, pass the DataTable into that class as input. The code that generates the word document should have nothing to do with the user interface.

If you follow some or all of my suggestions your code will look something like this, please note that this code identifies possible bugs in the code where return is not called.

    string fileName = @"G:\forwarding.docx";    // This assignment belong in the function that generates the word document


    if (kryptonDataGridView2.SelectedRows.Count > 0)
    {
        if (kryptonDataGridView2.SelectedRows.Count > 3)
        {
            MessageBox.Show(this, "Please select 3 or less Credit/Debit notes", "Selection Error", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
            return;
        }

        if (kryptonDataGridView1.SelectedRows.Count > 0 && kryptonDataGridView1.SelectedRows.Count > 8)

        {
            MessageBox.Show(this, "Please select atleast 1 bill and/or maximum 8 bills", "Selection Error", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
            // missing return statement here.
        }
        else
        {


            if ()// Call function here to calculate the totals here and return a boolean value
            {
                MessageBox.Show(this, "Credit/Debit note total value cannot be higher than total bill value", "Selection Error", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
                return;
            }

            decimal adj_total = total - _total;

            // call function to generate word document here. The if below belongs in the function that generates the document
            if (File.Exists(fileName))
            else

            MessageBox.Show(this, "Done", "Info", MessageBoxButtons.OK, MessageBoxIcon.Information);
            kryptonDataGridView1.Rows.Clear();
            kryptonDataGridView2.Rows.Clear();
        }
    }
    else
    {
        if (kryptonDataGridView1.SelectedRows.Count < 1 || kryptonDataGridView1.SelectedRows.Count > 15)
        {
            MessageBox.Show(this, "Please select minimum 1 item and/or maximum 15 items", "Selection Error", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
            // missing return here
        }
        else
        {
            // Call function to generate word document here
            MessageBox.Show(this, "Done", "Info", MessageBoxButtons.OK, MessageBoxIcon.Information);
        }
    }

Don't Repeat Yourself

This is in response to your comment/question at the end of your answer.

The art/science of software design is to keep breaking the problem into smaller and smaller pieces until each problem problem is very simple to resolve. I don't specifically know if you can merge those functions into one, but you will know if you keep breaking the functions into smaller pieces that can be reused.

Each one of the functions to generate repeats code that can be moved into functions, some examples are:

Formatting the First Paragraph

    private void FormatFirstPara(ref Xceed.Document.NET.Paragraph p1)
    {
        p1.FontSize(20).Append("W").Bold().Font("Arial").Color(Color.Red);
        p1.FontSize(20).Append("I").Bold().Font("Arial").Color(Color.Brown);
        p1.FontSize(20).Append("Z").Bold().Font("Arial").Color(Color.Gold);
        p1.FontSize(20).Append("D").Bold().Font("Arial").Color(Color.Chocolate);
        p1.FontSize(20).Append("E").Bold().Font("Arial").Color(Color.Teal);
        p1.FontSize(20).Append("R").Bold().Font("Arial").Color(Color.Violet);
        p1.FontSize(20).Append("M").Bold().Font("Arial").Color(Color.Orange);


        p1.Alignment = Alignment.center;
        p1.FontSize(20);
    }

Adding the cndn Table

    private void Cndn_table1()
    {
        cndn_table.Design = TableDesign.TableGrid;
        cndn_table.Alignment = Alignment.center;
        cndn_table.AutoFit = AutoFit.Contents;

        int _rowNumber = 0;

        int _columnNumber = 0;
        foreach (DataGridViewColumn column in kryptonDataGridView2.Columns)
        {
            cndn_table.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
            _columnNumber++;
        }

        _rowNumber++;

        foreach (DataGridViewRow row in kryptonDataGridView2.SelectedRows.OfType<DataGridViewRow>().OrderBy(c => c.Index))
        {
            _columnNumber = 0;
            foreach (DataGridViewColumn column in kryptonDataGridView2.Columns)
            {
                cndn_table.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(row.Cells[_columnNumber].Value.ToString());
                _columnNumber++;
            }

            _rowNumber++;
        }


        cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count + 1].Cells[2].Paragraphs.First().Append(_total.ToString()).Bold(true);
        cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count + 1].MergeCells(0, 1);

        cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count + 2].Cells[0].Paragraphs.First().Append("Total after adjustment :").Bold(true).Alignment = Alignment.right;
        cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count + 2].Cells[2].Paragraphs.First().Append(adj_total.ToString()).Bold(true);
        cndn_table.Rows[kryptonDataGridView2.SelectedRows.Count + 2].MergeCells(0, 1);
    }

Suggestions and Suggested Reading

Here are basic software engineering guidelines that universities teach and generally applied in the industry.

Design Patterns

Use design patterns. Many design patterns apply across multiple programming languages. Some well know design patterns that might apply to your projects are:

Both of these design patterns are used in C# with Windows Forms, they are also common web development design patterns.

DRY Code

There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code multiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.

Complexity

I disagree a bit with a previous review, this code is too complex, which means it does too much. This is only part of a function, not a complete function and it does not begin to fit on one screen in an editor or IDE. A common programming practice is that any function that is larger than a single screen is too complex and should be broken into smaller functions.

There is a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

SOLID Programming

SOLID is 5 object oriented design principles. SOLID is a mnemonic acronym for five design principles intended to make software designs more understandable, flexible and maintainable. This will help you design your objects and classes better.

  1. The Single Responsibility Principle - A class should only have a single responsibility, that is, only changes to one part of the software's specification should be able to affect the specification of the class.
  2. The Open–closed Principle - states software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
  3. The Liskov Substitution Principle - Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
  4. The Interface segregation principle - states that no client should be forced to depend on methods it does not use.
  5. The Dependency Inversion Principle - is a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details.

Example

You also might want to look at this code review question that takes data from an excel spreadsheet and generates word documents as an example, the data and logic have been separated out in this question.

\$\endgroup\$
5
\$\begingroup\$

Yes, this code is not easy and I don't have an easy answer for you. Only a couple tips.

The problem is not complexity, the problem is that the code is too long and overwhelming.

First of all, you have too many nested control blocks but you already know that. For instance the opening starts at line 4:

 if (kryptonDataGridView2.SelectedRows.Count > 0)
 {

and ends at line 279:

...    MessageBox.Show(this,"Done","Info",MessageBoxButtons.OK,MessageBoxIcon.Information);
             kryptonDataGridView1.Rows.Clear();
             kryptonDataGridView2.Rows.Clear();
         }
     }

Which is quite further down. So it's difficult to figure out where an if statement ends, especially when you nest them. The potential for confusion and bugs is very high.

But you can restructure your code to improve the situation. First of all, you can perform validation and return early if any of the validation steps fail. This is step 1.

And then, only proceed to step 2 if validation is successful. The idea is to reduce nesting, and better separate validation from data processing. Validation deserves a block of its own.

Naming: you should rename your DGVs. Don't use kryptonDataGridView1, kryptonDataGridView2, give them meaningful names instead, that reflect their purpose, namely bills and credit notes. That will make the code more clear and avoid bugs due to confusion or copy-paste.

You definitely need to break down your code in functions. For instance, the routine that generates the document should be a standalone function. That will make the code more manageable.

Something else that is badly needed is comments. You would do yourself a favor by commenting each step, even if it's just one line. And the comments would better delineate the numerous steps you are performing. Make it more visually appealing. Code that is hard to read is boring, tedious to maintain and invites more bugs.

In fact, even someone who knows the code (you) still has to scan and analyze the code to locate a specific action. A properly placed comment would allow you to spot the relevant code section instantly, and make it less of an eyesore.

At line 35 you check whether a document exists:

...
             if (File.Exists(fileName))
             {

                 using( var document = DocX.Load( fileName ) )
                 {

The matching else is at line 155:

...
             }
             else
             {
                 using( var document = DocX.Create( fileName ) )
                 {

Too much nesting, too much scrolling. This block should look more like:

if (File.Exists(fileName)) {
    LoadDocument(fileName, params);
} else {
    CreateDocument(fileName, params);
}

But you have an identical block repeated (from line 288). You should check only at one place that the file exists, and then branch accordingly.

And in fact there is a lot of repeated code, because loading or creating a new document involves more or less the same code. If you moved that part to a distinct function, the duplication would become more obvious. The code size can be reduced significantly, which will make it more readable and manageable.

A little experiment: if you fold the document generation code (that is duplicated and present at two places) here is how your program looks like (in Notepad++ but another IDE will do):

Code outline

As you can see very little remains, and the code suddenly looks more approachable and less daunting :) So my suggestion would be to remove the document generation code, restructure your ifs and write a dedicate function to generate the document.

\$\endgroup\$
1
  • \$\begingroup\$ I've posted an answer by adding dedicated functions separately from the main code to make the entire code more understandable (I hope). But there is still a lot of redundant code in them. How can I merge/update them to make it more efficient? Also, how can I structure the if statements better? \$\endgroup\$ Commented Jan 17, 2022 at 2:01
0
\$\begingroup\$

I've updated my code as per suggestion by @Kate & @pacmaninbw (created dedicated functions, reduced repeated code etc.)

    //create a function for generating the first para/header and add a single row table mentioning the party name (taken from combo box)
    public void FormatFirstPara(DocX doc)
    {
        Xceed.Document.NET.Header header_default = doc.Headers.Odd;
        Xceed.Document.NET.Paragraph p1 = header_default.InsertParagraph();
        p1.FontSize(20).Append("W").Bold().Font("Arial").Color(Color.Red);
        p1.FontSize(20).Append("I").Bold().Font("Arial").Color(Color.Brown);
        p1.FontSize(20).Append("Z").Bold().Font("Arial").Color(Color.Gold);
        p1.FontSize(20).Append("D").Bold().Font("Arial").Color(Color.Chocolate);
        p1.FontSize(20).Append("E").Bold().Font("Arial").Color(Color.Teal);
        p1.FontSize(20).Append("R").Bold().Font("Arial").Color(Color.Violet);
        p1.FontSize(20).Append("M").Bold().Font("Arial").Color(Color.Orange);
        
        
        p1.Alignment=Alignment.center;
        p1.FontSize(20);
        
        Xceed.Document.NET.Paragraph fdate=doc.InsertParagraph();
        fdate.Append("                                       Date:  "+DateTime.Today.ToString("dd-MMM-yyyy"));
        fdate.Alignment=Alignment.right;

        Xceed.Document.NET.Paragraph blank=doc.InsertParagraph();

        var tab = doc.AddTable( 1, 3 );
        tab.Design = TableDesign.TableGrid;
        tab.Alignment=Alignment.center;
        tab.Rows[0].MergeCells(0,2);
        tab.AutoFit=AutoFit.Contents;
        tab.Rows[0].Cells[0].Paragraphs.First().Append(kryptonComboBox1.Text).Bold().Alignment=Alignment.center;
        
        doc.InsertTable( tab );
    }
    
    //creating a table of bill data     
    public void OnlyBills_Table(Table bills)
    {
        bills.Design = TableDesign.TableGrid;
        bills.Alignment=Alignment.center;
        bills.AutoFit = AutoFit.Contents;
        
        int rowNumber = 0;

        int columnNumber = 0;
        foreach (DataGridViewColumn column in billsDGV.Columns)
        {
            bills.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
            columnNumber++;
        }

        rowNumber++;

        foreach (DataGridViewRow row in billsDGV.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
        {
            columnNumber = 0;
            foreach (DataGridViewColumn column in billsDGV.Columns)
            {
                bills.Rows[rowNumber].Cells[columnNumber].Paragraphs.First().Append(row.Cells[columnNumber].Value.ToString());
                columnNumber++;
            }

            rowNumber++;
        }
    }

    //creating a table of bill data with credit/debit notes     
    public void AdjustedBills_Table(Table bills)
    {
        bills.Design = TableDesign.TableGrid;
        bills.Alignment=Alignment.center;
        bills.AutoFit = AutoFit.Contents;
        
        int _rowNumber = 0;

        int _columnNumber = 0;
        foreach (DataGridViewColumn column in adjustmentbillsDGV.Columns)
        {
            bills.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(column.HeaderText).Bold();
            _columnNumber++;
        }

        _rowNumber++;

        foreach (DataGridViewRow row in adjustmentbillsDGV.SelectedRows.OfType<DataGridViewRow>().OrderBy(c=>c.Index))
        {
            _columnNumber = 0;
            foreach (DataGridViewColumn column in adjustmentbillsDGV.Columns)
            {
                bills.Rows[_rowNumber].Cells[_columnNumber].Paragraphs.First().Append(row.Cells[_columnNumber].Value.ToString());
                _columnNumber++;
            }

            _rowNumber++;
        }
        
        bills.Rows[adjustmentbillsDGV.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(_bl_total.ToString()).Bold(true);
        bills.Rows[adjustmentbillsDGV.SelectedRows.Count+1].MergeCells(0,1);
        
        bills.Rows[adjustmentbillsDGV.SelectedRows.Count+2].Cells[0].Paragraphs.First().Append("Total after adjustment :").Bold(true).Alignment=Alignment.right;
        bills.Rows[adjustmentbillsDGV.SelectedRows.Count+2].Cells[2].Paragraphs.First().Append(adj_bl_total.ToString()).Bold(true);
        bills.Rows[adjustmentbillsDGV.SelectedRows.Count+2].MergeCells(0,1);
        
    }
    
    decimal bl_total,_bl_total,adj_bl_total;

    //creating a method for document creation/updation using the above methods
    public void Modify_DocX(string fname)
    {
        bl_total = billsDGV.SelectedRows.OfType<DataGridViewRow>()
            .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));
        
        if (adjustmentbillsDGV.SelectedRows.Count > 0)
        {
            _bl_total = adjustmentbillsDGV.SelectedRows.OfType<DataGridViewRow>()
                .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));
            
            adj_bl_total=bl_total - _bl_total;
        }

        
        DocX document;
        if (File.Exists(fname))
        {
            document = DocX.Load( fname );
            document.AddHeaders();
            
            Xceed.Document.NET.Paragraph para=document.InsertParagraph();
            para.InsertPageBreakBeforeSelf();
        }
        else
        {
            document = DocX.Create( fname ) ;
            document.AddHeaders();
            
            Xceed.Document.NET.Paragraph para=document.InsertParagraph();
        }
        
        FormatFirstPara(document);

        Table table;
        
        if (adjustmentbillsDGV.SelectedRows.Count > 0)
        {
            table = document.AddTable( billsDGV.SelectedRows.Count+3, billsDGV.Columns.Count );
        }
        else
        {
            table = document.AddTable( billsDGV.SelectedRows.Count+2, billsDGV.Columns.Count );
        }
        
        OnlyBills_Table(table);

        if (adjustmentbillsDGV.SelectedRows.Count > 0)
        {
            table.Rows[billsDGV.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(bl_total.ToString()).Bold(true);
            table.Rows[billsDGV.SelectedRows.Count+1].MergeCells(0,1);
            
            table.Rows[billsDGV.SelectedRows.Count+2].Cells[0].Paragraphs.First().Append("Credit/Debit Note").Bold(true).Alignment=Alignment.center;
            table.Rows[billsDGV.SelectedRows.Count+2].MergeCells(0,2);
            
            document.InsertTable(table);
            
            var cndn_table = document.AddTable( adjustmentbillsDGV.SelectedRows.Count+3, adjustmentbillsDGV.Columns.Count );
            
            AdjustedBills_Table(cndn_table);

            document.InsertTable(cndn_table);
            
            document.Save();
        }
        else
        {
            table.Rows[billsDGV.SelectedRows.Count+1].Cells[0].Paragraphs.First().Append("Total :").Bold(true).Alignment=Alignment.right;
            table.Rows[billsDGV.SelectedRows.Count+1].Cells[2].Paragraphs.First().Append(bl_total.ToString()).Bold(true);
            table.Rows[billsDGV.SelectedRows.Count+1].MergeCells(0,1);

            document.InsertTable(table);
            
            document.Save();

        }
        
        document.Dispose();
    }

The main button code:

        string fileName=@"G: forwarding.docx";
                    
        if (adjustmentbillsDGV.SelectedRows.Count > 0)
        {
            if (adjustmentbillsDGV.SelectedRows.Count > 3)
            {
                MessageBox.Show(this,"Please select 3 or less Credit/Debit notes","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
                return;
            }
            
            
            if (billsDGV.SelectedRows.Count > 0 && billsDGV.SelectedRows.Count > 8)
            {
                MessageBox.Show(this,"Please select atleast 1 bill and/or maximum 8 bills","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
            }
            
            else
            {
                decimal total1 = billsDGV.SelectedRows.OfType<DataGridViewRow>()
                    .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));
                decimal _total1 = adjustmentbillsDGV.SelectedRows.OfType<DataGridViewRow>()
                    .Sum(tt => Convert.ToDecimal(tt.Cells[2].Value));

                if (_total1 > total1)
                {
                    MessageBox.Show(this,"Credit/Debit note total value cannot be higher than total bill value","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
                    return;
                }                    
                Modify_DocX(fileName);

                MessageBox.Show(this,"Done","Info",MessageBoxButtons.OK,MessageBoxIcon.Information);
                billsDGV.Rows.Clear();
                adjustmentbillsDGV.Rows.Clear();
                kryptonComboBox1.Text="";
            }
        }
        else
        {
            if (billsDGV.SelectedRows.Count < 1 || billsDGV.SelectedRows.Count > 15)
            {
                MessageBox.Show(this,"Please select minimum 1 item and/or maximum 15 items","Selection Error",MessageBoxButtons.OK,MessageBoxIcon.Exclamation);
            }
            else
            {
                Modify_DocX(fileName);
        MessageBox.Show(this,"Done","Info",MessageBoxButtons.OK,MessageBoxIcon.Information);
                billsDGV.Rows.Clear();
                adjustmentbillsDGV.Rows.Clear();
                kryptonComboBox1.Text="";
            }
        }
\$\endgroup\$
5

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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