1

I'm trying to convert this into an async method, but I don't think I'm querying the data properly to do it. What am I missing to:

  1. make this cleaner
  2. make it async

All the InventorySelection is, is a class with the various filter items.

private IEnumerable<T_IFS_Inventory> Getquery(T_IFS_InventorySelectDTO InventorySelection)
{
    IEnumerable<T_IFS_Inventory> query = _db.T_IFS_Inventory.AsQueryable();

    if (InventorySelection.SerialNumber != "" && InventorySelection.SerialNumber != null)
    {
        query = query.Where(q => q.SerialNumber == InventorySelection.SerialNumber);
    }
    else
    {
        if (InventorySelection.ScanDateStart != null)
        {
            query = query.Where(q => q.ScanDate >= InventorySelection.ScanDateStart);

            if (InventorySelection.ScanDateEnd != null)
                query = query.Where(q => q.ScanDate <= InventorySelection.ScanDateEnd);
        }

        if (InventorySelection.StatusDateStart != null)
        {
            query = query.Where(q => q.DateStatus >= InventorySelection.StatusDateStart);

            if (InventorySelection.StatusDateEnd != null)
                query = query.Where(q => q.DateStatus <= InventorySelection.StatusDateEnd);
        }

        if (InventorySelection.CategoryID != 0)
            query = query.Where(q => q.CategoryID == InventorySelection.CategoryID);

        if (InventorySelection.BrandID != 0)
            query = query.Where(q => q.BrandID == InventorySelection.BrandID);

        if (InventorySelection.ModelID != 0)
            query = query.Where(q => q.ModelID == InventorySelection.ModelID);

        if (InventorySelection.EmployeeID != 0)
            query = query.Where(q => q.EmployeeID == InventorySelection.EmployeeID);

        if (InventorySelection.StatusID != 0)
            query = query.Where(q => q.StatusID == InventorySelection.StatusID);

        if (InventorySelection.EmployeeStatusID != 0)
            query = query.Where(q => q.EmployeeStatusID == InventorySelection.EmployeeStatusID);

        if (InventorySelection.CurrentLocationID != 0)
            query = query.Where(q => q.CurrentLocationID == InventorySelection.CurrentLocationID);
    }

    return query;
}
9
  • 1
    "Convert this into an async method" in what way? What asynchronous operation are you trying to perform? What did you try and what didn't work as expected? Commented Jan 12, 2023 at 20:28
  • A query is just an expression of intent. Any asynchronicity comes from the execution of the query, not the construction of it. Commented Jan 12, 2023 at 20:31
  • Unrelated: use string.IsNullOrEmpty. Small change but very convenient. Commented Jan 12, 2023 at 20:35
  • @David Well, it is doing DB so I thought it would be base async. Commented Jan 12, 2023 at 20:39
  • I'd probably consider a builder pattern with fluent api for that. But would depend on usage. I also created a WhereIf extension method on IQueryable, which also was quite handy. Commented Jan 12, 2023 at 20:39

1 Answer 1

3

Your query is being executed at the first line, I hope this is a typo, but IEnumerable is wrong as the variable type. It explicitly forces ALL of the records to be loaded into memory before any of your filters are applied. What you want is to defer the execution until all of the criteria have been defined.

IQueryable<T_IFS_Inventory> query = _db.T_IFS_Inventory.AsQueryable();

With only this change the query will be evaluated and the data will be returned as part of the return process. Because the method return type is IEnumerable the change from IQueryable to IEnumerable would force the evaluation as part of the return process.

To make this async, deliberately resolve the query:

return query.ToListAsync();

Or you can await it if you need this method's line info in the stack trace:

return await query.ToListAsync();

Both of these changes will require the method prototype to be made async too:

private async Task<IEnumerable<T_IFS_Inventory>> Getquery(T_IFS_InventorySelectDTO InventorySelection)

However now the method isn't getting a query it is now executing the entire query and bringing all of the data into memory.

A better approach that will allow you to compose on top of this query would be to change the method prototype to return IQueryable

private async IQueryable<T_IFS_Inventory> Getquery(T_IFS_InventorySelectDTO InventorySelection)

Ideally, your method should look like this:

private IQueryable<T_IFS_Inventory> Getquery(T_IFS_InventorySelectDTO InventorySelection)
{
    IQueryable<T_IFS_Inventory> query = _db.T_IFS_Inventory.AsQueryable();

    if (!String.IsNullOrEmpty(InventorySelection.SerialNumber))
    {
        query = query.Where(q => q.SerialNumber == InventorySelection.SerialNumber);
    }
    else
    {
        if (InventorySelection.ScanDateStart != null)
        {
            query = query.Where(q => q.ScanDate >= InventorySelection.ScanDateStart);

            if (InventorySelection.ScanDateEnd != null)
                query = query.Where(q => q.ScanDate <= InventorySelection.ScanDateEnd);
        }

        if (InventorySelection.StatusDateStart != null)
        {
            query = query.Where(q => q.DateStatus >= InventorySelection.StatusDateStart);

            if (InventorySelection.StatusDateEnd != null)
                query = query.Where(q => q.DateStatus <= InventorySelection.StatusDateEnd);
        }

        if (InventorySelection.CategoryID != 0)
            query = query.Where(q => q.CategoryID == InventorySelection.CategoryID);

        if (InventorySelection.BrandID != 0)
            query = query.Where(q => q.BrandID == InventorySelection.BrandID);

        if (InventorySelection.ModelID != 0)
            query = query.Where(q => q.ModelID == InventorySelection.ModelID);

        if (InventorySelection.EmployeeID != 0)
            query = query.Where(q => q.EmployeeID == InventorySelection.EmployeeID);

        if (InventorySelection.StatusID != 0)
            query = query.Where(q => q.StatusID == InventorySelection.StatusID);

        if (InventorySelection.EmployeeStatusID != 0)
            query = query.Where(q => q.EmployeeStatusID == InventorySelection.EmployeeStatusID);

        if (InventorySelection.CurrentLocationID != 0)
            query = query.Where(q => q.CurrentLocationID == InventorySelection.CurrentLocationID);
    }

    return query;
}

It is important when we use this query builder pattern that you keep the query as an IQueryable until all of the criteria have been added.

In terms of code style, I'd call this very clean, anything else is going to affect the readability, right now it's very easy to follow.

Sign up to request clarification or add additional context in comments.

8 Comments

Mind that it is recommended and common practice to append "Async" to the method name and passing a cancellationtoken.
That's a very 1980s mindset. Unless you are going to implement both an async and non async version of this method then it is redundant information that is available by virtue of the response type being a Task<>. Yes there are common linting guidance that enforces this, but my goal is to keep the code clean and let my IDE give me hints when I need them. This method is also called GetQuery and now has two responsibilities, get the query and execute it to return the data. Async in the name is the least concern.
@ChrisSchaller, getting closer but is says 'IEnumerable<T_IFS_Inventory>' does not contain a definition for 'ToListAsync'. ToList works but is not async.
Since it is called "GetQuery" I'd actually return the IQueryable, yes. And then you can forget above convention.
@SteveCross I missed the fact that your variable was IEnumerable... It should be IQueryable! I've updated my response, please read it again from the top
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.