abbas
abbas

Reputation: 432

Optimize this LINQ Query

I am having performance issues with this LINQ Query. The data is laoded into to this.students already. Now when I call the GetStudentData function say 1000 times it has a huge overhead. Is there a way of improving this without changing the LINQ to a loop

   public Student GetStudentData()
   {         
          IEnumerable<Students> studentTypes = this.students.Where(x => (x.studentsId  == studentId && x.StduentType.Equals(studentType)));
          if(studentTypes.Count==0) return new Student() { studentid=studentID};
          return (Student)studentTypes.First();
   }

So here are the results when looping through it 10000 times with my original version

Original Version : 5.6 seconds on the average New Version @des's Code with FirstOrDefault : 3.6 seconds

Upvotes: 1

Views: 177

Answers (3)

Branko Dimitrijevic
Branko Dimitrijevic

Reputation: 52157

Refactor your code so this.students is a Dictionary<int, Student> (key is StudentId), then reimplement your method similarly to this:

public Student GetStudentData(int studentId, StudentType studentType) {
    Student result;
    if (this.students.TryGetValue(studentId, out result))
        if (result.StudentType.Equals(studentType)))
            return result;
    return new Student { StudentId = studentId };
}

If you absolutely can't refactor this.students, you can always maintain the dictionary in parallel.

Or, you can simply create a temporary dictionary (this.students.ToDictionary(student => student.StudentId)) just before the 1000-iteration loop.

Upvotes: 0

Zbigniew
Zbigniew

Reputation: 27614

When you use Where you loop through all records which fulfill given conditions, when you use First you just search for first record which fullfills condition, so using First should speed it up.

public Student GetStudentData()
{         
    // get first student by id and type, return null if there is no such student
    var student = students.FirstOrDefault(i => i.studentsId == studentId && i.StudentType.Equals(studentType));

    // if student is null then return new student
    return student ?? new Student();
}

Upvotes: 4

Icarus
Icarus

Reputation: 63970

Well, the issue is precisely the fact that you are calling this method in a loop, supposedly, 1000 times!

Why not changing the method to receive a list of studentIDs and return the 1000 students in one shot? Something like

var studentTypes = from c in this.students 
                   where studentIDs.Contains(c.StudentID)
                   select c;

Where studentIDs can be an int[] containing the list of student ids you want.

Upvotes: 2

Related Questions