0

I have this method where I am saving data into the database after doing a bit of evaluation. Within the foreach iteration I check whether conditions of are met before an object is assigned values then further inserted into the database. When the method runs it would only save one instance of eligibleSupplier into the database or throw IDENTITY_INSERT exception

public void EvaluationTenderBids(int tenderId)
    {
        var tender = _databaseContext.Tenders.Find(tenderId);
        var submittedTenders = _databaseContext.TenderBidSubmissions.Where(t => t.TenderId == tenderId).ToList();
        EligibleSupplier eligibleSupplier = new EligibleSupplier();
        tender.EligibleSuppliers = new List<EligibleSupplier>();


        var eligibleSuppliers = new List<EligibleSupplier>();

        decimal totProductQtAmt = 0;
        decimal totProductRecAmt = 0;
        decimal priceDifference = 0;
        
        
        
        foreach (var tenderBid in submittedTenders)
        {

            var tenderBidProducts = _databaseContext.TenderBidSubmissionProducts.Where(t => t.TenderBidSubmissionId == tenderBid.TenderBidSubmissionId).ToList();

            foreach(var product in tenderBidProducts)
            {
                var prod = _databaseContext.Products.Find(product.ProductId);
                product.RecommendedPrice = prod.ProductPrice;
                totProductQtAmt = product.QuotedPrice * product.Quantity;
                totProductRecAmt = product.RecommendedPrice * product.Quantity;

                priceDifference = totProductQtAmt - totProductRecAmt;
            }

            var company = _databaseContext.TenderBidSubmissions.Where(c => c.RegistrationNumber == tenderBid.RegistrationNumber).FirstOrDefault();

            decimal percentage = 0;

            if (priceDifference > 0)
            {
                percentage = ((priceDifference / tenderBid.TotalQuotation) * 100);
            }


            if (percentage > 27 && percentage <= 30)
            {
                eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
                eligibleSupplier.CompanyName = company.CompanyName;
                eligibleSupplier.Tender = tender;
                eligibleSupplier.TenderId = tender.TenderId;
                eligibleSupplier.Score = 1;
                eligibleSupplier.InflationRate = percentage;
                eligibleSupplier.DateEvaluated = DateTime.Now;

                tender.EligibleSuppliers.Add(eligibleSupplier);
                _databaseContext.SaveChanges();

            }
            else if (percentage > 21 && percentage <= 24)
            {
                eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
                eligibleSupplier.CompanyName = company.CompanyName;
                eligibleSupplier.Tender = tender;
                eligibleSupplier.TenderId = tender.TenderId;
                eligibleSupplier.Score = 2;
                eligibleSupplier.InflationRate = percentage;
                eligibleSupplier.DateEvaluated = DateTime.Now;

                tender.EligibleSuppliers.Add(eligibleSupplier);
                _databaseContext.SaveChanges();
            }
            else if (percentage > 18 && percentage < 21)
            {
                eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
                eligibleSupplier.CompanyName = company.CompanyName;
                eligibleSupplier.Tender = tender;
                eligibleSupplier.TenderId = tender.TenderId;
                eligibleSupplier.Score = 3;
                eligibleSupplier.InflationRate = percentage;
                eligibleSupplier.DateEvaluated = DateTime.Now;

                tender.EligibleSuppliers.Add(eligibleSupplier);
                _databaseContext.SaveChanges();
            }
            else if (percentage > 15 && percentage <= 18)
            {
                eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
                eligibleSupplier.CompanyName = company.CompanyName;
                eligibleSupplier.Tender = tender;
                eligibleSupplier.TenderId = tender.TenderId;
                eligibleSupplier.Score = 4;
                eligibleSupplier.InflationRate = percentage;
                eligibleSupplier.DateEvaluated = DateTime.Now;

                tender.EligibleSuppliers.Add(eligibleSupplier);
                _databaseContext.SaveChanges();
            }
            else if (percentage > 10 && percentage <= 15)
            {
                eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
                eligibleSupplier.CompanyName = company.CompanyName;
                eligibleSupplier.Tender = tender;
                eligibleSupplier.TenderId = tender.TenderId;
                eligibleSupplier.Score = 5;
                eligibleSupplier.InflationRate = percentage;
                eligibleSupplier.DateEvaluated = DateTime.Now;


                tender.EligibleSuppliers.Add(eligibleSupplier);
                _databaseContext.SaveChanges();
            }
            else
                continue;
        
        }

    }
Prince Ntombela
  • 21
  • 5

1 Answers1

0

Ok, there is a considerable number of problems with your code to go over:

#1. With entities, never do this:

var tender = _databaseContext.Tenders.Find(tenderId);
tender.EligibleSuppliers = new List<EligibleSupplier>();

When EF loads an entity, you want it to manage the child collections. This means avoiding trying to "reset" a collection with a new instance. When loading an existing tender you should be eager-loading the collection of suppliers then determining whether you want to remove/replace, or edit existing rows, before adding new ones.

Find is only really useful when you know you only need the data in an entity and none of the related entities, unless you want to handle manually loading them on demand or rely on lazy loading. Instead, you should use:

var tender = _databaseContext.Tenders
    .Include(x => x.EligibleSuppliers)
    .Single(x => x.TenderId == tenderId);

This will load a single tender by ID and eager load any suppliers currently recorded. From there you need to decide whether you want to update the values of any existing EligibleSuppliers, or remove them (I.e. use tender.EligibleSuppliers.Clear()) and re-add new entities.

#2. EF respects references. Don't use the same reference for multiple entities. This code here:

// Outside of a loop...
EligibleSupplier eligibleSupplier = new EligibleSupplier();

// Inside a loop...
eligibleSupplier.InflationRate = percentage;
eligibleSupplier.DateEvaluated = DateTime.Now;

tender.EligibleSuppliers.Add(eligibleSupplier);

This is 99.9% surely the cause of your immediate issues. When you add a reference to a supplier, then re-use that reference you are updating the instance that is already added to the supplier.

Inside a loop if you want to create/add a new supplier you need to declare a new instance, not reuse the same instance.

The same issue will be affecting your priceDifference calculation. It is only going to respect the value of the last product in the product loop because of the line in the loop:

priceDifference = totProductQtAmt - totProductRecAmt;

This should be something like:

priceDifference += totProductQtAmt - totProductRecAmt;

if you want to know if the total price of all ordered products differ. (I.e. some products more may still cancel out products that are less)

#3. Overly repetitive conditional code. Your whole if / else if / else if are all doing the exact same thing except changing the Score.

All of this:

        if (percentage > 27 && percentage <= 30)
        {
            eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
            eligibleSupplier.CompanyName = company.CompanyName;
            eligibleSupplier.Tender = tender;
            eligibleSupplier.TenderId = tender.TenderId;
            eligibleSupplier.Score = 1;
            eligibleSupplier.InflationRate = percentage;
            eligibleSupplier.DateEvaluated = DateTime.Now;

            tender.EligibleSuppliers.Add(eligibleSupplier);
            _databaseContext.SaveChanges();

        }
        else if (percentage > 21 && percentage <= 24)
        {
            eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
            eligibleSupplier.CompanyName = company.CompanyName;
            eligibleSupplier.Tender = tender;
            eligibleSupplier.TenderId = tender.TenderId;
            eligibleSupplier.Score = 2;
            eligibleSupplier.InflationRate = percentage;
            eligibleSupplier.DateEvaluated = DateTime.Now;

            tender.EligibleSuppliers.Add(eligibleSupplier);
            _databaseContext.SaveChanges();
        }
        else if (percentage > 18 && percentage < 21)
        {
            eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
            eligibleSupplier.CompanyName = company.CompanyName;
            eligibleSupplier.Tender = tender;
            eligibleSupplier.TenderId = tender.TenderId;
            eligibleSupplier.Score = 3;
            eligibleSupplier.InflationRate = percentage;
            eligibleSupplier.DateEvaluated = DateTime.Now;

            tender.EligibleSuppliers.Add(eligibleSupplier);
            _databaseContext.SaveChanges();
        }
        else if (percentage > 15 && percentage <= 18)
        {
            eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
            eligibleSupplier.CompanyName = company.CompanyName;
            eligibleSupplier.Tender = tender;
            eligibleSupplier.TenderId = tender.TenderId;
            eligibleSupplier.Score = 4;
            eligibleSupplier.InflationRate = percentage;
            eligibleSupplier.DateEvaluated = DateTime.Now;

            tender.EligibleSuppliers.Add(eligibleSupplier);
            _databaseContext.SaveChanges();
        }
        else if (percentage > 10 && percentage <= 15)
        {
            eligibleSupplier.RegistrationNumber = company.RegistrationNumber;
            eligibleSupplier.CompanyName = company.CompanyName;
            eligibleSupplier.Tender = tender;
            eligibleSupplier.TenderId = tender.TenderId;
            eligibleSupplier.Score = 5;
            eligibleSupplier.InflationRate = percentage;
            eligibleSupplier.DateEvaluated = DateTime.Now;


            tender.EligibleSuppliers.Add(eligibleSupplier);
            _databaseContext.SaveChanges();
        }
        else
            continue;

can be replaced with a helper method: (see: Is there a "between" function in C#?)

public static bool Between(this int num, int lower, int upper, bool inclusive = false)
{
    return inclusive
        ? lower <= num && num <= upper
        : lower < num && num < upper;
}

and then: (Assuming you have a typo in your logic as the first set was 27-30 while the second set was 21-24, I doubt you mean to ignore 25-26...

int score = percentage.Between(24, 30) ? 1
    : percentage.Between(21, 24) ? 2
    : percentage.Between(18, 21) ? 3
    : percentage.Between(15, 18) ? 4
    : percentage.Between(10, 15) ? 5
    : 0;

if (score == 0)
    continue;

var eligibleSupplier = new EligibleSupplier
{
     RegistrationNumber = company.RegistrationNumber;
     CompanyName = company.CompanyName;
     Score = score;
     InflationRate = percentage;
     DateEvaluated = DateTime.Now;
};

tender.EligibleSuppliers.Add(eligibleSupplier);

When adding a Supplier to a Tender's collection, you don't need to set the EligibleSupplier.Tender or .TenderId, EF will take care of that automatically. It's generally harmless to do it, just unnecessary. Setting FK fields (I.e. TenderId) should always be avoided where navigation properties are in play. This can lead to inconsistent state depending on whether the navigation property is loaded or not at the time.

Steve Py
  • 26,149
  • 3
  • 25
  • 43