patterncsharpMinor

Retrieve Tasks by Employee

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

retrievetasksemployee

Problem

I think I've done a decent job keeping this query simple and understandable, but I'd like to know if you have some more advice.

My relevant entities are defined like this:

public class Task
{
    [ForeignKey("Employee")]
    public string EmployeeId { get; set; }
    public Employee Employee { get; set; }

    [Required]
    [ForeignKey("Status")]
    public int StatusId { get; set; }
    public TaskStatus Status { get; set; }

    [...]
}

public class Employee
{
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.None)]
    [Required]
    [MaxLength(10)]
    [Index(IsUnique=true)]
    public string EmployeeId { get; set; }

    [Required]
    [StringLength(100)]
    public string FirstName { get; set; }

    [Required]
    [StringLength(100)]
    [DisplayName("Last name")]
    public string LastName { get; set; }

    [...]
}

public class TaskStatus
{
    [Key]
    public int Id { get; set; }

    [Required]
    [MaxLength(50)]
    public string Name { get; set; }

    [...]
}

I've stripped off what I believe are irrelevant parts of the entities.

The query itself is following:

```
// Retrieves all tasks from the database. We filter the tasks on 3 different criteria. Since all 3 criteria
// must be fulfilled for the task to be displayed, we use logical and to concatenate them.
//
// The first criteria is the search term. If the search term is empty, we don't filter out any tasks, so
// that is the first check we make. Also, we test if the FistName, LastName or the EmployeeId contain the
// search term.
//
// Next criteria is the status filter. We keep the tasks that have the right status, or all tasks if the
// given status is null.
//
// The last criteria is employee filter, and the logic is the same as for the status filter.
var tasks = db.Tasks
.Include(t => t.Employee)
.Include(t => t.Status)
.Where(t =>
(
search == null
|| t.Employee.FirstName.Contains(search)
|| t.Employee.LastName

Solution

Remove the additional ()s in the statusFilter and employeeFilter lines as they are unnecessary.

Indent the statusFilter and employeeFilter lines so that they are symmetrical with the search lines.

Change search to be searchTerm as it is more specific. search by itself could mean anything like a stored query, a boolean value, etc.

As per Magus' comment you can change t to task since it is more clear and will not add space that starts pushing statements across extra lines.

var tasks = db.Tasks
  .Include(task => task.Employee)
  .Include(task => task.Status)
  .Where(task =>
    (
      searchTerm == null || 
      task.Employee.FirstName.Contains(searchTerm) ||
      task.Employee.LastName.Contains(searchTerm) ||
      task.Employee.EmployeeId.Contains(searchTerm)
    ) &&
    (
      statusFilter == null || 
      task.StatusId = statusFilter.Value
    ) &&
    (
      employeeFilter == null || 
      task.EmployeeId == employeeFilter
    )
);

Code Snippets

var tasks = db.Tasks
  .Include(task => task.Employee)
  .Include(task => task.Status)
  .Where(task =>
    (
      searchTerm == null || 
      task.Employee.FirstName.Contains(searchTerm) ||
      task.Employee.LastName.Contains(searchTerm) ||
      task.Employee.EmployeeId.Contains(searchTerm)
    ) &&
    (
      statusFilter == null || 
      task.StatusId = statusFilter.Value
    ) &&
    (
      employeeFilter == null || 
      task.EmployeeId == employeeFilter
    )
);

Context

StackExchange Code Review Q#47250, answer score: 2

Revisions (0)

No revisions yet.