0
\$\begingroup\$

I have a object list like below. I want to join every two rows into single row based on column B. It is sure that only two rows would be there for every single column B value.

Input

enter image description here

Output

enter image description here

However, my solution works. but I am looking for more better solution. I am not much happy with my solution.

My solution:

var groupByItems = items.GroupBy(x => x.ColumnB).Select(x => new MappingClass
    {
        ColumnA= x.FirstOrDefault().ColumnA,
        ColumnB= x.FirstOrDefault().ColumnB,
        ColumnC= x.Where(r=> !string.IsNullOrEmpty(r.ColumnC)).Select(r=>r.ColumnC).FirstOrDefault(),
        ColumnD= x.Where(r => !string.IsNullOrEmpty(r.ColumnD)).Select(r => r.ColumnD).FirstOrDefault(),
    }).ToList();

Now groupByItems object returns me two rows as expected.

\$\endgroup\$
2
  • 4
    \$\begingroup\$ Welcome to the Code Review Community. Can you provide more context about this code in the question. Why are you merging multiple rows? What does the rest of the program do? Why do you want to improve this code, is it too slow, using too much memory? It is very hard to help you improve the code without more context. \$\endgroup\$
    – pacmaninbw
    Apr 25, 2021 at 12:33
  • \$\begingroup\$ IMHO Your input nor your output make sense WRT ColumnA. Why would they be the same for all lines in Input or in Output, and where is the Output value supposed to be coming from? Also, what is your input? Because if is is the result of a DB call, you might consider a custom query instead. \$\endgroup\$
    – BCdotWEB
    Apr 25, 2021 at 18:05

1 Answer 1

1
\$\begingroup\$

If you want to reduce the iteration to get the value for ColumnC and ColumnD and make the code cleaner, you can try using Aggregate instead.

var output = input
             .GroupBy(d => new {d.ColumnA, d.ColumnB})
             .Select(
                 g => g.Aggregate(
                     new MappingClass {ColumnA = g.Key.ColumnA, ColumnB = g.Key.ColumnB},
                     (result, next) =>
                     {
                         result.ColumnC ??= next.ColumnC;
                         result.ColumnD ??= next.ColumnD;
                         return result;
                     }))
              .ToList();
\$\endgroup\$
3
  • \$\begingroup\$ +1 neat. you've just forgot to add ToList() to match OP final result. ;). Also, I think you will get the same results without new MappingClass { .... } line. \$\endgroup\$
    – iSR5
    Apr 25, 2021 at 16:22
  • \$\begingroup\$ @iSR5 thanks for noticing that, I've updated the code. And the problem with removing the new MappingClass is that you're modifying the input data in the enumerable operation which is not a good practice. \$\endgroup\$
    – Moch Yusup
    Apr 26, 2021 at 3:10
  • \$\begingroup\$ totally missed the reference type behavior ;).. thanks for the explanation \$\endgroup\$
    – iSR5
    Apr 26, 2021 at 16:01

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