2
\$\begingroup\$

I've been looking over internet for a method of combining 2 datatables with exactly the same cells with numbers to add them together, Merge method never help because it just appended the second table. The task was later accomplished by my hand-written method:

static DataTable CombineDT34(DataTable dt3, DataTable dt4) {
        var combinedDT34 = dt3;
        for (var r3 = 0; r3 < dt3.Rows.Count; r3++) {

            for(var c3 = 1; c3 < dt3.Columns.Count; c3++) {

                dt3.Rows[r3][c3] = 
                    (Convert.ToInt32(dt3.Rows[r3][c3]) +
                    Convert.ToInt32(dt4.Rows[r3][c3]));
            }
        }

        return dt3;
    }

Is there are a way to do this better?

\$\endgroup\$
  • \$\begingroup\$ Is it known that the two tables are the same size? \$\endgroup\$ – 200_success Sep 21 '17 at 13:58
  • \$\begingroup\$ kinda, but they have the same columns \$\endgroup\$ – Sibongeleni Mdlalose Sep 22 '17 at 9:22
1
\$\begingroup\$

Naming of methods and variables

I don't like them. What is dt3? What is dt4? Why are you calling the variable r3 and c3? Why not name them by what they represent? Also the name of the method I feel, is not clear.

Secondly I must agree with @Paparazzi:

  • there is no count to make sure that the tables are equivalent. You should probably add some exception handling there and also - you are indeed starting at column 1 which is well spotted.
  • Table3 will be passed by reference so you are actually changing table three.
  • I've copied the table, thereby not changing table 3 itself. And i've also added a method to make it a little more readable.

Comments on the Code below:

  • I've changed the variable and method names to make them a little more intuitive - but you should probably change table3 and table4 to names which more accurately represent what those tables actually are. Also your code above assumes we are dealing with a table of integers.

    static DataTable CombineTable3With4(DataTable table3, DataTable table4)
    {
        var combinedTable = table3.Copy();
    
        for (var rowNumber = 0; rowNumber < table3.Rows.Count; rowNumber++)
        {
            for (var columnNumber = 1; columnNumber < table3.Columns.Count; columnNumber++)
            {
                CombineRows(combinedTable, table4, rowNumber, columnNumber);
            }
        }
    
        return combinedTable;
    }
    
    private static void CombineRows(DataTable combinedTable, DataTable table4, int rowNumber, int columnNumber)
    {
        combinedTable.Rows[rowNumber][columnNumber] = (Convert.ToInt32(combinedTable.Rows[rowNumber][columnNumber]) + Convert.ToInt32(table4.Rows[rowNumber][columnNumber]));
    }
    
\$\endgroup\$
  • \$\begingroup\$ Definately, but problem is I'm new here. How you add a reputation? \$\endgroup\$ – Sibongeleni Mdlalose Sep 22 '17 at 12:58
  • \$\begingroup\$ Click on the up arrow to the left of the answer. You should upvote any answers that you find helpful, and any questions that you find interesting. That’s how thanks are expressed in this community, and the reputation points are what make the site work. If you haven’t already, take the tour. \$\endgroup\$ – 200_success Sep 22 '17 at 13:10
  • \$\begingroup\$ This is not working code. \$\endgroup\$ – paparazzo Sep 22 '17 at 13:10
  • 1
    \$\begingroup\$ There are still 2 bugs. columnNumber = 1 and you are mutating table3 and yet you are not returning the mutated table3. And you have the accepted answer. \$\endgroup\$ – paparazzo Sep 22 '17 at 15:40
  • \$\begingroup\$ @Paparazzi - thx have fixed the bug - but have left column = 1 untouched because that was in OP's code.. (i can't choose the accepted answer but i have clearly attributed your influence in the above post) \$\endgroup\$ – BKSpurgeon Sep 22 '17 at 15:46
1
\$\begingroup\$

You are not testing for same size.

You don't use combinedDT34. You are changing an input which is not a good practice. I am not sure if dt3 will be passed by reference.

You are starting column on 1.

static DataTable CombineDT34(DataTable dt3, DataTable dt4) 
{
    if(dt3.Rows.Count != dt4.Rows.Count)
        throw new ArgumentOutOfRangeException);
    //...

    for (var r3 = 0; r3 < dt3.Rows.Count; r3++) 
    {
        for(var c3 = 0; c3 < dt3.Columns.Count; c3++) 
        {
            dt3.Rows[r3][c3] = ( Convert.ToInt32(dt3.Rows[r3][c3]) +
                                 Convert.ToInt32(dt4.Rows[r3][c3]) );
        }
    }
    return dt3;
}
\$\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.