5
\$\begingroup\$

I have Products table which looks like this:

enter image description here

I have some data in this table and I want to filter it depending on this columns (Filtering with columns type of string isn't problem). For this I have the code (in ASP.NET Core Web API Project) shown below. I know it's not a good approach. I have another table which has more foreign keys, so it will be more difficult to do this kind of work on it.

In SearchProduct action method I am getting the product object, which properties have specific values. For example, If Name="Box", then I want to get products from the product list, which Name is "Box" too.

The consumer of this service is WPF application, where I have the window, where the controls (textboxes and comboboxes{ for productType and ProductUnit}) are placed and the user will be able to search data with any criteria. For example, if he wants to search only with Name, he will write something only in Name textbox. If he wants to search product which name is "Box" and also the productType is "A", then he will write "Box" in name textbox and will choose A type in comboBox.

P.S When product is added in this table, if user doesn't write anything in the Name textbox for example, the value of Name will be "" and not null.

    [HttpPost("SearchProduct")]
    public IEnumerable<Product> SearchProduct([FromBody]Product product)
    {
        IEnumerable<Product> data;

        if(product.ProductTypeID != null && product.ProductUnitID != null)
        {
            data = _context.Products.Where(x =>
          x.Name.Contains(product.Name) &&
          x.Code.Contains(product.Code) &&
          x.Barcode.Contains(product.Barcode) &&
          x.InnerCode.Contains(product.InnerCode) &&
          (x.ProductTypeID == product.ProductTypeID &&
          x.ProductUnitID == product.ProductUnitID))

           .Include(x => x.ProductType)
           .Include(x => x.ProductUnit);
        }
        else if(product.ProductTypeID == null && product.ProductUnitID == null)
        {
            data = _context.Products.Where(x =>
        x.Name.Contains(product.Name) &&
        x.Code.Contains(product.Code) &&
        x.Barcode.Contains(product.Barcode) &&
        x.InnerCode.Contains(product.InnerCode) ||
        (x.ProductTypeID == product.ProductTypeID &&
        x.ProductUnitID == product.ProductUnitID))

         .Include(x => x.ProductType)
         .Include(x => x.ProductUnit);
        }
        else if(product.ProductTypeID != null)
        {
            data = _context.Products.Where(x =>
        x.Name.Contains(product.Name) &&
        x.Code.Contains(product.Code) &&
        x.Barcode.Contains(product.Barcode) &&
        x.InnerCode.Contains(product.InnerCode) &&
        x.ProductTypeID == product.ProductTypeID)

         .Include(x => x.ProductType)
         .Include(x => x.ProductUnit);
        }
        else
        {
            data = _context.Products.Where(x =>
        x.Name.Contains(product.Name) &&
        x.Code.Contains(product.Code) &&
        x.Barcode.Contains(product.Barcode) &&
        x.InnerCode.Contains(product.InnerCode) &&
        x.ProductUnitID == product.ProductUnitID)

         .Include(x => x.ProductType)
         .Include(x => x.ProductUnit);
        }



        return data;
    }
\$\endgroup\$
  • \$\begingroup\$ One more thing... this is you actual code, isn't? It's a bit strange that you don't have something like .ToList() anywhere. The context should already be disposed when the query is executed... I think this would throw an excepiton. \$\endgroup\$ – t3chb0t Jul 28 '18 at 6:26
  • 1
    \$\begingroup\$ Yes, this is my actual code. I used scaffolding to get the controller actions and ToList() wasn't included there. Exceptions aren't thrown too. But this is only in async method. Maybe it has difference? \$\endgroup\$ – Beginner Jul 28 '18 at 6:29
2
\$\begingroup\$

You code seems to contradict your text. You say:

For example, if he wants to search only with Name, he will write something only in Name textbox. If he wants to search product which name is "Box" and also the productType is "A", then he will write "Box" in name textbox and will choose A type in comboBox.

And yet each method seems to contain multiple Where clauses.

Wouldn't it be more flexible to construct an IQueryable<T>? Something along these lines:

private IQueryable<Product> GetProductsQueryable(Product request)
{
    var queryable = _dbContext.Products.AsQueryable();

    if (!string.IsNullOrEmpty(request.Name))
    {
        queryable = queryable.Where(x => x.Name.Contains(request.Name));
    }

    if (!string.IsNullOrEmpty(request.Code))
    {
        queryable = queryable.Where(x => x.Code.Contains(request.Code));
    }

    if (!string.IsNullOrEmpty(request.Barcode))
    {
        queryable = queryable.Where(x => x.Barcode.Contains(request.Barcode));
    }

    return queryable;
}
\$\endgroup\$
1
\$\begingroup\$

You should maybe reconsider the fields in the data table. You should restrict nullable fields to a minimum. A product without a name or ProductTypeId?


As for the queries, you could simplify it to not repeat code like:

public List<Product> SearchProduct([FromBody]Product product)
{
  IEnumerable<Product> data = _context.Products.Where(x =>
    x.Name.Contains(product.Name) &&
    x.Code.Contains(product.Code) &&
    x.Barcode.Contains(product.Barcode) &&
    x.InnerCode.Contains(product.InnerCode));

  if (product.ProductTypeID == null && product.ProductUnitID == null)
  {
    data = data.Concat(data.Where(p => p.ProductTypeID == null && p.ProductUnitID == null));
  }
  else
  {
    if (product.ProductTypeID != null)
    {
      data = data.Where(p => p.ProductTypeID == product.ProductTypeID);
    }
    if (product.ProductUnitID != null)
    {
      data = data.Where(p => p.ProductUnitID == product.ProductUnitID);
    }
  }

  return data.Include(x => x.ProductType).Include(x => x.ProductUnit).ToList();
}

As commented by t3chb0t the query is executed while leaving the method so that you know that the DbContext is still valid.

\$\endgroup\$
  • \$\begingroup\$ How can I change structure of my tables? what's your opinion? \$\endgroup\$ – Beginner Jul 28 '18 at 16:09
  • \$\begingroup\$ @Beginner: It depends on many things that are unknown from your code. But I just wonder, how a product can be nameless (name = NULL). Further, for the int fields, I would make them non-nullable and maybe have a neutral (0) value for missing values. \$\endgroup\$ – Henrik Hansen Jul 29 '18 at 5:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.