CODEWITHSUNDEEP
c#lambda
Shailendra Mishra
Shailendra Mishra

Reputation: 71

Can this lambda expression become more simple?

var MaleCount = students.Where(Std => Std.Gender.ToUpper() == "M").Count();
var FemaleCount = students.Where(Std => Std.Gender.ToUpper() == "F").Count();

//List for storing top students records           
List<StudentEntity> TopStudents = new List<StudentEntity>();

//Adding records to List
if (MaleCount > 0)
{
    var maxMarksM = students.Where(o => o.Gender.ToUpper() == "M").Max(o => o.Marks);
    TopStudents = students.Where(o => o.Gender.ToUpper() == "M" && o.Marks == maxMarksM).ToList();
}
if (FemaleCount > 0)
{
    var maxMarksF = students.Where(o => o.Gender.ToUpper() == "F").Max(o => o.Marks);
    TopStudents.AddRange(students.Where(o => o.Gender.ToUpper() == "F" && o.Marks == maxMarksF).ToList());
}

return TopStudents;

Upvotes: 1

Views: 131

Answers (2)

Stefan Rother-St&#252;bs
Stefan Rother-St&#252;bs

Reputation: 363

Disclaimer: I program in Groovy, but that should not make a difference in this case.

If you don't want to use the chained .GroupBy().Where()... solution maybe that way:

Split students into maleStudents and femaleStudents (using Where()).

This should remove the need for the if-wrappers and for Count() around the lines for male and female students.

So my alternative should look like this:

var MaleStudents = students.Where(Std => Std.Gender.ToUpper() == "M");
var FemaleStudents = students.Where(Std => Std.Gender.ToUpper() == "F");

//List for storing top students records           
List<StudentEntity> TopStudents = new List<StudentEntity>();

//Adding records to List
var maxMarksM = MaleStudents.Max(o => o.Marks);
TopStudents = MaleStudents.Where(o => o.Marks == maxMarksM).ToList();

var maxMarksF = FemaleStudents.Max(o => o.Marks);
TopStudents.AddRange(FemaleStudents.Where(o => o.Marks == maxMarksF).ToList());

return TopStudents;

Upvotes: 0

areller
areller

Reputation: 5238

var topStudents = allStudents
                .GroupBy(s => s.Gender.ToUpper()) // Dividing the students to groups by gender
                .Where(g => g.Key == "M" || g.Key == "F") // Including only the Male and Female genders
                .Select(g => g.Where(s => s.Marks == g.Max(i => i.Marks))) // From each group, finding the highest mark and selecting from that groups all the student with that mark
                .SelectMany(s => s) // selecting all the students from all the inner groups
                .ToList();

Edit:

@Alexey Subbota suggested that g.Max might get called too many times, in fact it will be called one time for every student inside the group, which is unnecessary, we only need to compute the maximum once for every group. If it's an issue, you could do this:

var topStudents = allStudents
                .GroupBy(s => s.Gender.ToUpper()) // Dividing the students to groups by gender
                .Where(g => g.Key == "M" || g.Key == "F") // Including only the Male and Female genders
                .Select(g => new KeyValuePair<int, IEnumerable<Student>>(g.Max(s => s.Marks), g)) // Storing the group together with it's highest score value
                .Select(g => g.Value.Where(s => s.Marks == g.Key)) // From each group, selecting the student that have the highest score
                .SelectMany(s => s) // selecting all the students from all the inner groups
                .ToList();

Upvotes: 6

Related Questions