patterncsharpMinor
Group by and sum query on multiple columns
Viewed 0 times
groupcolumnsquerymultiplesumand
Problem
var fpslist = db.FPSinformations.Where(x => x.Godown_Code != null && x.Godown_Code == godownid).ToList();
var data1 = fpslist.GroupBy(x => x.Ration_Card_Type1)
.Select(x => new
{
CardType_Name = x.Key,
CardType_Count = x.Sum(y => y.Ration_Card_Count1)
}).ToList();
var data2 = fpslist.GroupBy(x => x.Ration_Card_Type2)
.Select(x => new
{
CardType_Name = x.Key,
CardType_Count = x.Sum(y => y.Ration_Card_Count2)
}).ToList();
var data3 = fpslist.GroupBy(x => x.Ration_Card_Type3)
.Select(x => new
{
CardType_Name = x.Key,
CardType_Count = x.Sum(y => y.Ration_Card_Count3)
}).ToList();
var data4 = fpslist.GroupBy(x => x.Ration_Card_Type4)
.Select(x => new
{
CardType_Name = x.Key,
CardType_Count = x.Sum(y => y.Ration_Card_Count4)
}).ToList();
var data5 = fpslist.GroupBy(x => x.Ration_Card_Type5)
.Select(x => new
{
CardType_Name = x.Key,
CardType_Count = x.Sum(y => y.Ration_Card_Count5)
}).ToList();
var GodownRCCount = data1.Where(x => x.CardType_Name != null).ToList();
var GodownRCCounts = GodownRCCount;
GodownRCCount = data2.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);
GodownRCCount = data3.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);
GodownRCCount = data4.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);
GodownRCCount = data5.Where(x => x.CardType_Name != null).ToList();
GodownRCCounts.AddRange(GodownRCCount);I have 10 columns in my database like this:
Ration_Card_Type1
Ration_card_count1
Ration_Card_Type2
Ration_card_count2
Ration_Card_Type3
Ration_card_count3
Ration_Card_Type4
Ration_card_count4
Ration_Card_Type5
Ration_card_count5
Now what I want is to get the sum of
Ration_Card_Counts and its type from its type.Expected output:
CardType_Name
CardType_Count
It works fine, but I want to optimize
Solution
First and most obvious issue is usage of
Unless comparison operator for
Now let's make a reusable function (and also avoid to create multiple anonymous types, it's a micro optimization but it also simplifies our code):
Now your code can be greatly simplified (I assumed you cannot change database columns otherwise everything can be even simpler, see last section of this answer). I also used generics because type of your columns is unknown.
Note that from your code it seems you can have multiple rows with a value for
Note that you can consume
About DB
I do not know if you can change your database schema and I do not know exact layout of your data then this section may not apply in your case.
If your data is in the form:
Ration_Card_Type1 Ration_Card_Count1 Ration_Card_Type2 Ration_Card_Count2
Type1 5 NULL 0
NULL 0 Type2 11
Type1 12 Type2 3
Then you have to keep your actual code but I would suggest to do not map 1:1 column names with C# property names (conventions are pretty different) then
If, for example, you have data with this simplified layout:
Ration_Card_Type1 Ration_Card_Count1 Ration_Card_Type2 Ration_Card_Count2
Type1 5 NULL 0
NULL 0 Type2 11
Type1 12 NULL 0
Then you may both optimize and simplify your DB (it will also impact code):
Type Count
1 5
2 11
1 12
In that case all you need (assuming your C# entities maps 1:1 with DB columns):
ToList(), it's not required and you force creation of multiple (and possibly big) lists moreover you're repeating almost same code again and again. Step by step:var fpslist = db.FPSinformations.Where(x => x.Godown_Code == godownid);Unless comparison operator for
Godown_Code is broken then you shouldn't need to check for null (in case you may opt for ?. to avoid it). Also you don't need .ToList(), drop it. If you will execute such code inside a loop you may consider to move this outside the loop (to be sure more context is needed).Now let's make a reusable function (and also avoid to create multiple anonymous types, it's a micro optimization but it also simplifies our code):
static IEnumerable> CountByType(
IEnumerable list,
Func nameSelector,
Func, TCount> aggregator)
{
return list.Where(x => Object.Equals(nameSelector(x), null) == false)
.GroupBy(x => nameSelector(x))
.Select(x => Tuple.Create(x.Key, aggregator(x)));
}Now your code can be greatly simplified (I assumed you cannot change database columns otherwise everything can be even simpler, see last section of this answer). I also used generics because type of your columns is unknown.
var data1 = CountByType(fpslist,
x => x.Ration_Card_Type1,
x => x.Sum(y => y.Ration_Card_Count1));
var data2 = CountByType(fpslist,
x => x.Ration_Card_Type2,
x => x.Sum(y => y.Ration_Card_Count2));
// Repeat for each set
var result = data1
.Concatenate(data2)
.Concatenate(data3)
.Concatenate(data4)
.Concatenate(data5);Note that from your code it seems you can have multiple rows with a value for
Ration_Card_CountX. If you have just one row then all code will be greatly reduced. Here I used a Tuple because I do not know exact types of your objects, you'd better use a proper class instead.Note that you can consume
result as-is (an enumeration) or materialize a list (don't forget this if data comes from DB and connection will be disposed, LINQ execution is deferred).About DB
I do not know if you can change your database schema and I do not know exact layout of your data then this section may not apply in your case.
If your data is in the form:
Ration_Card_Type1 Ration_Card_Count1 Ration_Card_Type2 Ration_Card_Count2
Type1 5 NULL 0
NULL 0 Type2 11
Type1 12 Type2 3
Then you have to keep your actual code but I would suggest to do not map 1:1 column names with C# property names (conventions are pretty different) then
Ration_Card_Type1 should be RationCardType1 (assuming you can't simply have Type1, Type2 and so on).If, for example, you have data with this simplified layout:
Ration_Card_Type1 Ration_Card_Count1 Ration_Card_Type2 Ration_Card_Count2
Type1 5 NULL 0
NULL 0 Type2 11
Type1 12 NULL 0
Then you may both optimize and simplify your DB (it will also impact code):
Type Count
1 5
2 11
1 12
In that case all you need (assuming your C# entities maps 1:1 with DB columns):
var result = db.GroupBy(x => x.Type)
.Select(x => new { Type = x.Key, Count = x.Sum(y => y.Count) });Code Snippets
var fpslist = db.FPSinformations.Where(x => x.Godown_Code == godownid);static IEnumerable<Tuple<TName, TCount>> CountByType<T, TName, TCount>(
IEnumerable<T> list,
Func<T, TName> nameSelector,
Func<IEnumerable<T>, TCount> aggregator)
{
return list.Where(x => Object.Equals(nameSelector(x), null) == false)
.GroupBy(x => nameSelector(x))
.Select(x => Tuple.Create(x.Key, aggregator(x)));
}var data1 = CountByType(fpslist,
x => x.Ration_Card_Type1,
x => x.Sum(y => y.Ration_Card_Count1));
var data2 = CountByType(fpslist,
x => x.Ration_Card_Type2,
x => x.Sum(y => y.Ration_Card_Count2));
// Repeat for each set
var result = data1
.Concatenate(data2)
.Concatenate(data3)
.Concatenate(data4)
.Concatenate(data5);var result = db.GroupBy(x => x.Type)
.Select(x => new { Type = x.Key, Count = x.Sum(y => y.Count) });Context
StackExchange Code Review Q#113896, answer score: 3
Revisions (0)
No revisions yet.