3
\$\begingroup\$

I have a few lines where I am trying to get a list of the 25 most popular Items sold on an E-Commerce web site. The code works but I would like hints on how to optimize it.

var ecommerceItems = await _context.BmaEcItems
                    .Select(e => e.Itemnmbr)
                    .ToListAsync();

IList<PopularItem> popularItems = new List<PopularItem>();
foreach (var item in ecommerceItems)
{
    var pi = new PopularItem
    {
        ItemNumber = item,
        QuantitySold = await _context.OrderDetails.Where(e => e.Itemnmbr == item).SumAsync(e => Convert.ToInt32(e.Quantity))
    };
    popularItems.Add(pi);
}

var itemList = popularItems.OrderByDescending(e => e.QuantitySold).Select(e => e.ItemNumber).Take(25).ToList();

BmaEcItems is a list of all items sold via the E-Commerce web site (It is a subset of a master inventory list). OrderDetails is a list of the details of the sales orders (This is a list of all item details sold and includes items not on the BmaEcItems list).

Any hints are appreciated.

UPDATE

As requested:

public partial class BmaEcItem : INotifyPropertyChanged
{
    public string Itemnmbr { get; set; }
    public string ExtendedDesc { get; set; }
    public bool? Featured { get; set; }
    public string Metadata1 { get; set; }
    public string Metadata2 { get; set; }
    public string Metadata3 { get; set; }
    public string Metadata4 { get; set; }
    public string Metadata5 { get; set; }
    public string Metadata6 { get; set; }
    public int DefaultStockCount { get; set; }
    public bool? DisplayOnEcommerce { get; set; }
    public string Metadata7 { get; set; }
    public string Metadata8 { get; set; }
    public string Metadata9 { get; set; }
    public string Metadata10 { get; set; }
    public string Metadata11 { get; set; }
    public string Metadata12 { get; set; }
    public string Metadata13 { get; set; }
    public string Metadata14 { get; set; }
    public string Metadata15 { get; set; }
    public string Metadata16 { get; set; }
    public string Metadata17 { get; set; }
    public string Metadata18 { get; set; }
    public string Metadata19 { get; set; }
    public string Metadata20 { get; set; }

    public event PropertyChangedEventHandler PropertyChanged;

    [NotifyPropertyChangedInvocator]
    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}
[Table("SOP10200")]
public class OrderDetail : INotifyPropertyChanged
{
    [Key]
    [Column("SOPTYPE")]
    [EnumDataType(typeof(OrderType))]
    public OrderType Soptype { get; set; }

    [Key]
    [Column("SOPNUMBE")]
    [StringLength(21)]
    public string Sopnumbe { get; set; }
    [Key]
    [Column("LNITMSEQ")]
    public int Lnitmseq { get; set; }
    [Key]
    [Column("CMPNTSEQ")]
    public int Cmpntseq { get; set; }
    [Required]
    [Column("ITEMNMBR")]
    [StringLength(31)]
    public string Itemnmbr { get; set; }
    [Required]
    [Column("ITEMDESC")]
    [StringLength(101)]
    public string Itemdesc { get; set; }
    [Column("NONINVEN")]
    public short Noninven { get; set; }
    [Column("DROPSHIP")]
    public short Dropship { get; set; }
    [Required]
    [Column("UOFM")]
    [StringLength(9)]
    public string Uofm { get; set; }
    [Required]
    [Column("LOCNCODE")]
    [StringLength(11)]
    public string Locncode { get; set; }
    [Column("UNITCOST", TypeName = "numeric(19, 5)")]
    public decimal Unitcost { get; set; }
    [Column("ORUNTCST", TypeName = "numeric(19, 5)")]
    public decimal Oruntcst { get; set; }
    [Column("UNITPRCE", TypeName = "numeric(19, 5)")]
    public decimal Unitprce { get; set; }
    [Column("ORUNTPRC", TypeName = "numeric(19, 5)")]
    public decimal Oruntprc { get; set; }
    [Column("XTNDPRCE", TypeName = "numeric(19, 5)")]
    public decimal Xtndprce { get; set; }
    [Column("OXTNDPRC", TypeName = "numeric(19, 5)")]
    public decimal Oxtndprc { get; set; }
    [Column("REMPRICE", TypeName = "numeric(19, 5)")]
    public decimal Remprice { get; set; }
    [Column("OREPRICE", TypeName = "numeric(19, 5)")]
    public decimal Oreprice { get; set; }
    [Column("EXTDCOST", TypeName = "numeric(19, 5)")]
    public decimal Extdcost { get; set; }
    [Column("OREXTCST", TypeName = "numeric(19, 5)")]
    public decimal Orextcst { get; set; }
    [Column("MRKDNAMT", TypeName = "numeric(19, 5)")]
    public decimal Mrkdnamt { get; set; }
    [Column("ORMRKDAM", TypeName = "numeric(19, 5)")]
    public decimal Ormrkdam { get; set; }
    [Column("MRKDNPCT")]
    public short Mrkdnpct { get; set; }
    [Column("MRKDNTYP")]
    public short Mrkdntyp { get; set; }
    [Column("INVINDX")]
    public int Invindx { get; set; }
    [Column("CSLSINDX")]
    public int Cslsindx { get; set; }
    [Column("SLSINDX")]
    public int Slsindx { get; set; }
    [Column("MKDNINDX")]
    public int Mkdnindx { get; set; }
    [Column("RTNSINDX")]
    public int Rtnsindx { get; set; }
    [Column("INUSINDX")]
    public int Inusindx { get; set; }
    [Column("INSRINDX")]
    public int Insrindx { get; set; }
    [Column("DMGDINDX")]
    public int Dmgdindx { get; set; }
    [Required]
    [Column("ITMTSHID")]
    [StringLength(15)]
    public string Itmtshid { get; set; }
    [Column("IVITMTXB")]
    public short Ivitmtxb { get; set; }
    [Column("BKTSLSAM", TypeName = "numeric(19, 5)")]
    public decimal Bktslsam { get; set; }
    [Column("ORBKTSLS", TypeName = "numeric(19, 5)")]
    public decimal Orbktsls { get; set; }
    [Column("TAXAMNT", TypeName = "numeric(19, 5)")]
    public decimal Taxamnt { get; set; }
    [Column("ORTAXAMT", TypeName = "numeric(19, 5)")]
    public decimal Ortaxamt { get; set; }
    [Column("TXBTXAMT", TypeName = "numeric(19, 5)")]
    public decimal Txbtxamt { get; set; }
    [Column("OTAXTAMT", TypeName = "numeric(19, 5)")]
    public decimal Otaxtamt { get; set; }
    [Column("BSIVCTTL")]
    public byte Bsivcttl { get; set; }
    [Column("TRDISAMT", TypeName = "numeric(19, 5)")]
    public decimal Trdisamt { get; set; }
    [Column("ORTDISAM", TypeName = "numeric(19, 5)")]
    public decimal Ortdisam { get; set; }
    [Column("DISCSALE", TypeName = "numeric(19, 5)")]
    public decimal Discsale { get; set; }
    [Column("ORDAVSLS", TypeName = "numeric(19, 5)")]
    public decimal Ordavsls { get; set; }
    [Column("QUANTITY", TypeName = "numeric(19, 5)")]
    public decimal Quantity { get; set; }
    [Column("ATYALLOC", TypeName = "numeric(19, 5)")]
    public decimal Atyalloc { get; set; }
    [Column("QTYINSVC", TypeName = "numeric(19, 5)")]
    public decimal Qtyinsvc { get; set; }
    [Column("QTYINUSE", TypeName = "numeric(19, 5)")]
    public decimal Qtyinuse { get; set; }
    [Column("QTYDMGED", TypeName = "numeric(19, 5)")]
    public decimal Qtydmged { get; set; }
    [Column("QTYRTRND", TypeName = "numeric(19, 5)")]
    public decimal Qtyrtrnd { get; set; }
    [Column("QTYONHND", TypeName = "numeric(19, 5)")]
    public decimal Qtyonhnd { get; set; }
    [Column("QTYCANCE", TypeName = "numeric(19, 5)")]
    public decimal Qtycance { get; set; }
    [Column("QTYCANOT", TypeName = "numeric(19, 5)")]
    public decimal Qtycanot { get; set; }
    [Column("QTYONPO", TypeName = "numeric(19, 5)")]
    public decimal Qtyonpo { get; set; }
    [Column("QTYORDER", TypeName = "numeric(19, 5)")]
    public decimal Qtyorder { get; set; }
    [Column("QTYPRBAC", TypeName = "numeric(19, 5)")]
    public decimal Qtyprbac { get; set; }
    [Column("QTYPRBOO", TypeName = "numeric(19, 5)")]
    public decimal Qtyprboo { get; set; }
    [Column("QTYPRINV", TypeName = "numeric(19, 5)")]
    public decimal Qtyprinv { get; set; }
    [Column("QTYPRORD", TypeName = "numeric(19, 5)")]
    public decimal Qtyprord { get; set; }
    [Column("QTYPRVRECVD", TypeName = "numeric(19, 5)")]
    public decimal Qtyprvrecvd { get; set; }
    [Column("QTYRECVD", TypeName = "numeric(19, 5)")]
    public decimal Qtyrecvd { get; set; }
    [Column("QTYREMAI", TypeName = "numeric(19, 5)")]
    public decimal Qtyremai { get; set; }
    [Column("QTYREMBO", TypeName = "numeric(19, 5)")]
    public decimal Qtyrembo { get; set; }
    [Column("QTYTBAOR", TypeName = "numeric(19, 5)")]
    public decimal Qtytbaor { get; set; }
    [Column("QTYTOINV", TypeName = "numeric(19, 5)")]
    public decimal Qtytoinv { get; set; }
    [Column("QTYTORDR", TypeName = "numeric(19, 5)")]
    public decimal Qtytordr { get; set; }
    [Column("QTYFULFI", TypeName = "numeric(19, 5)")]
    public decimal Qtyfulfi { get; set; }
    [Column("QTYSLCTD", TypeName = "numeric(19, 5)")]
    public decimal Qtyslctd { get; set; }
    [Column("QTYBSUOM", TypeName = "numeric(19, 5)")]
    public decimal Qtybsuom { get; set; }
    [Column("EXTQTYAL", TypeName = "numeric(19, 5)")]
    public decimal Extqtyal { get; set; }
    [Column("EXTQTYSEL", TypeName = "numeric(19, 5)")]
    public decimal Extqtysel { get; set; }
    [Column(TypeName = "datetime")]
    public DateTime ReqShipDate { get; set; }
    [Column("FUFILDAT", TypeName = "datetime")]
    public DateTime Fufildat { get; set; }
    [Column("ACTLSHIP", TypeName = "datetime")]
    public DateTime Actlship { get; set; }
    [Required]
    [Column("SHIPMTHD")]
    [StringLength(15)]
    public string Shipmthd { get; set; }
    [Required]
    [Column("SALSTERR")]
    [StringLength(15)]
    public string Salsterr { get; set; }
    [Required]
    [Column("SLPRSNID")]
    [StringLength(15)]
    public string Slprsnid { get; set; }
    [Required]
    [Column("PRCLEVEL")]
    [StringLength(11)]
    public string Prclevel { get; set; }
    [Required]
    [Column("COMMNTID")]
    [StringLength(15)]
    public string Commntid { get; set; }
    [Column("BRKFLD1")]
    public short Brkfld1 { get; set; }
    [Column("BRKFLD2")]
    public short Brkfld2 { get; set; }
    [Column("BRKFLD3")]
    public short Brkfld3 { get; set; }
    [Column("CURRNIDX")]
    public short Currnidx { get; set; }
    [Required]
    [Column("TRXSORCE")]
    [StringLength(13)]
    public string Trxsorce { get; set; }
    [Required]
    [Column("SOPLNERR")]
    [MaxLength(4)]
    public byte[] Soplnerr { get; set; }
    [Column("ORGSEQNM")]
    public int Orgseqnm { get; set; }
    [Required]
    [Column("ITEMCODE")]
    [StringLength(15)]
    public string Itemcode { get; set; }
    [Column("PURCHSTAT")]
    public short Purchstat { get; set; }
    [Column("DECPLQTY")]
    public short Decplqty { get; set; }
    [Column("DECPLCUR")]
    public short Decplcur { get; set; }
    [Column("ODECPLCU")]
    public short Odecplcu { get; set; }
    [Column("QTYTOSHP", TypeName = "numeric(19, 5)")]
    public decimal Qtytoshp { get; set; }
    [Column("XFRSHDOC")]
    public byte Xfrshdoc { get; set; }
    [Column("EXCEPTIONALDEMAND")]
    public byte Exceptionaldemand { get; set; }
    [Required]
    [Column("TAXSCHID")]
    [StringLength(15)]
    public string Taxschid { get; set; }
    [Column("TXSCHSRC")]
    public short Txschsrc { get; set; }
    [Required]
    [Column("PRSTADCD")]
    [StringLength(15)]
    public string Prstadcd { get; set; }
    [Required]
    [StringLength(65)]
    public string ShipToName { get; set; }
    [Required]
    [Column("CNTCPRSN")]
    [StringLength(61)]
    public string Cntcprsn { get; set; }
    [Required]
    [Column("ADDRESS1")]
    [StringLength(61)]
    public string Address1 { get; set; }
    [Required]
    [Column("ADDRESS2")]
    [StringLength(61)]
    public string Address2 { get; set; }
    [Required]
    [Column("ADDRESS3")]
    [StringLength(61)]
    public string Address3 { get; set; }
    [Required]
    [Column("CITY")]
    [StringLength(35)]
    public string City { get; set; }
    [Required]
    [Column("STATE")]
    [StringLength(29)]
    public string State { get; set; }
    [Required]
    [Column("ZIPCODE")]
    [StringLength(11)]
    public string Zipcode { get; set; }
    [Required]
    [Column("CCode")]
    [StringLength(7)]
    public string Ccode { get; set; }
    [Required]
    [Column("COUNTRY")]
    [StringLength(61)]
    public string Country { get; set; }
    [Required]
    [Column("PHONE1")]
    [StringLength(21)]
    public string Phone1 { get; set; }
    [Required]
    [Column("PHONE2")]
    [StringLength(21)]
    public string Phone2 { get; set; }
    [Required]
    [Column("PHONE3")]
    [StringLength(21)]
    public string Phone3 { get; set; }
    [Required]
    [Column("FAXNUMBR")]
    [StringLength(21)]
    public string Faxnumbr { get; set; }
    public short Flags { get; set; }
    [Column(TypeName = "numeric(19, 5)")]
    public decimal BackoutTradeDisc { get; set; }
    [Column(TypeName = "numeric(19, 5)")]
    public decimal OrigBackoutTradeDisc { get; set; }
    [Required]
    [Column("GPSFOINTEGRATIONID")]
    [StringLength(31)]
    public string Gpsfointegrationid { get; set; }
    [Column("INTEGRATIONSOURCE")]
    public short Integrationsource { get; set; }
    [Required]
    [Column("INTEGRATIONID")]
    [StringLength(31)]
    public string Integrationid { get; set; }
    [Required]
    [Column("CONTNBR")]
    [StringLength(11)]
    public string Contnbr { get; set; }
    [Column("CONTLNSEQNBR", TypeName = "numeric(19, 5)")]
    public decimal Contlnseqnbr { get; set; }
    [Column("CONTSTARTDTE", TypeName = "datetime")]
    public DateTime Contstartdte { get; set; }
    [Column("CONTENDDTE", TypeName = "datetime")]
    public DateTime Contenddte { get; set; }
    [Required]
    [Column("CONTITEMNBR")]
    [StringLength(31)]
    public string Contitemnbr { get; set; }
    [Required]
    [Column("CONTSERIALNBR")]
    [StringLength(21)]
    public string Contserialnbr { get; set; }
    [Column("BULKPICKPRNT")]
    public byte Bulkpickprnt { get; set; }
    [Column("INDPICKPRNT")]
    public byte Indpickprnt { get; set; }
    [Column("ISLINEINTRA")]
    public byte Islineintra { get; set; }
    [Required]
    [Column("SOFULFILLMENTBIN")]
    [StringLength(15)]
    public string Sofulfillmentbin { get; set; }
    [Column("MULTIPLEBINS")]
    public byte Multiplebins { get; set; }
    [Column("Print_Phone_NumberGB")]
    public short PrintPhoneNumberGb { get; set; }
    [Column("DEX_ROW_TS", TypeName = "datetime")]
    public DateTime DexRowTs { get; set; }
    [Column("DEX_ROW_ID")]
    public int DexRowId { get; set; }

    public Order Order { get; set; }
    public event PropertyChangedEventHandler PropertyChanged;

    [NotifyPropertyChangedInvocator]
    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}
\$\endgroup\$
5
  • \$\begingroup\$ Can you show the data models that used in the method? \$\endgroup\$
    – aepot
    Commented Jan 21, 2022 at 21:02
  • 1
    \$\begingroup\$ @aepot I have updated my question... \$\endgroup\$
    – Randy
    Commented Jan 21, 2022 at 21:31
  • \$\begingroup\$ Do you use some database? Which one? PostGreSQL or Sqlite? Something else e.g. Redis ? \$\endgroup\$ Commented Jan 22, 2022 at 10:16
  • \$\begingroup\$ @BasileStarynkevitch I am on MS SqlServer 2019 \$\endgroup\$
    – Randy
    Commented Jan 22, 2022 at 10:18
  • \$\begingroup\$ @Randy Well the sad truth is that ORM systems are designed for CRUD operations not for reporting queries. If performance does matter for you then should implement your query inside a stored procedure and call it via the Entity Framework. \$\endgroup\$ Commented Jan 25, 2022 at 11:55

2 Answers 2

3
\$\begingroup\$

When ToList is called, the IQueryable will be executed at DBMS, and store the results in the memory. So, if there is more data manipulation needed, it should be treated as part of the query, and before it's going to be stored in memory.

Current code will do unnecessary round-trip executions while it can be done in one query (executed once). To do so, you could use Join or adjust the models to add the proper navigation properties (Foreign Keys) and use them in the query.

When you add the proper relations between entities, EF will try to also add some indexes based on them. However, it's recommended to review, modify, add the proper indexes manually as much as needed. As the auto-indexing is not enough.

Here is an example on how to do it in one query using JOIN :

var popularItems = await _context.BmaEcItems
                    .Join(_context.OrderDetails,
                        items => items.Itemnmbr,
                        orders=> orders.Itemnmbr,
                        (items, orders) => new 
                        {
                            ItemNumber = items.Itemnmbr,
                            QuantitySold = orders.Sum(s=> s.Quantity)
                        )
                        .OrderByDescending(e => e.QuantitySold)                 
                        .Take(25)           
                        .ToListAsync();


var itemList = await popularItems.Select(e => e.ItemNumber).ToListAsync();

The above example will send a query to the database similar to this one :

SELECT TOP 25 *
FROM (
    SELECT 
        items.Itemnmbr       AS ItemNumber
    ,   SUM(orders.Quantity) AS QuantitySold
    FROM 
        BmaEcItems items
    JOIN OrderDetails orders ON items.Itemnmbr = orders.Itemnmbr
    GROUP BY 
        items.Itemnmbr
) e 
ORDER BY QuantitySold DESC

You can add the .Select(e => e.ItemNumber) after the Take(25) to include within the query, however, I preferred to return the ItemNumber and QuantitySold as re-useable code solution (so it can be reused elsewhere).

Finally, I recommend to use SQL Profiler, to monitor the code queries, while debugging, to help improve querying performance in your application.

\$\endgroup\$
0
\$\begingroup\$

Sorry for the late post. I did find a solution. The Foreach loop in my original question was making a database call for each item. It worked but it was painfully slow. My biggest improvement was getting that done in one database call with a group by.

var ecItems = await _context.BmaEcItems
    .Where(e => e.DisplayOnEcommerce == true)
    .Select(e => e.Itemnmbr)
    .ToListAsync();

var test = await _context.OrderDetails
    .Where(e => ecItems.Contains(e.Itemnmbr))
    .GroupBy(e => e.Itemnmbr)
    .Select(e => new { ItemNumber = e.Key, QuantitySold = Convert.ToInt32(e.Sum(x => x.Quantity)) })
    .OrderByDescending(e => e.QuantitySold)
    .Take(25)
    .ToListAsync();

var itemList = test.Select(e => e.ItemNumber)
    .ToList();
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thanks for posting this code. It's a good idea to summarise which changes you made, and why - a self-answer ought to review the code, just like any other answer. \$\endgroup\$ Commented May 12, 2022 at 15:57

Your Answer

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

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