I created this method, I have a list of bankNotes {1, 5, 10, 20, 100} each has 10 notes,

Everything in my code works fine but it seems like it is not efficient enough when calculating the notes and remaining inventory.

Can you please suggest how I can improve this code? Also, is using LINQ not efficient?

 public List<BankNotesEntity> GetNotes()
    {
        bankNotes = new List<BankNotesEntity>();
        bankNotes.AddRange(new List<BankNotesEntity>
        {
            new BankNotesEntity(1, 10),
            new BankNotesEntity(5, 10),
            new BankNotesEntity(10, 10),
            new BankNotesEntity(20, 10),
            new BankNotesEntity(100, 10),
        });
        return bankNotes;
    }
   
    public List<BankNotesEntity> DispenseNotes(int notesToDispense)
    {
        var input = notesToDispense;
        var inventoryNotes = bankNotes.OrderByDescending(n => n.Denomination).ToList();

        //Check if the inventory is sufficient enough to dispense notes
        var sum = inventoryNotes.Sum(s => s.Denomination * s.Inventory);
        var dispensedNotes = new List<BankNotesEntity>();

        if (sum < notesToDispense)
            dispensedNotes = new List<BankNotesEntity>();
        else
        {
            foreach (var invt in inventoryNotes)
            {
                var bankNoteEntity = new BankNotesEntity();
                bankNoteEntity.Denomination = invt.Denomination;
                var bill = invt.Denomination;
                while (input >= bill)
                {
                    if (invt.Inventory == 0)
                        break;
                    else
                    {
                        input -= bill;
                        invt.Inventory -= 1;
                        bankNoteEntity.Inventory  = 1;
                    }
                }
                dispensedNotes.Add(bankNoteEntity);
            }

            //counter check to make sure that the total dispesedNotes is equal to amountToDispensed
            var sumOfDispensedNotes = dispensedNotes.Sum(s => s.Denomination * s.Inventory);
            if (sumOfDispensedNotes < notesToDispense)
            {
                //dipensedNotes is not equal to amountToDispensed, now we will return it to the inventory.
                dispensedNotes.ForEach(delegate (BankNotesEntity d)
                {
                    var inventory = inventoryNotes.Find(i => i.Denomination == d.Denomination);
                    if (inventory != null)
                        inventory.Inventory  = d.Inventory;
                });
                dispensedNotes = new List<BankNotesEntity>();
            }
        }
        return dispensedNotes.OrderBy(n => n.Denomination).ToList();
    }

What I' trying to do is if I pass an amount to the method called

Example: DispenseNotes($275)

Code will check the list of denomination and dispense note from Higher Bill down to smaller bill.

Result would be the the denomination and it's available inventory.

Inventory:
    $1,10
    $5,9
    $10,9
    $20,7
    $100,8

For this inventory it is already updated and the notes already dispense as it's satisfy the condition where $275 is available to dispense given the list of inventory.

CodePudding user response：

Your implementation is quite verbose and I didn't really go through it.

Here's one way of implementing your requirement.

    public static void CalculateDenominations(int amount)
    {
        List<Inventory> inventory = new List<Inventory> 
        {
            new Inventory(100, 10),
            new Inventory(20, 10),
            new Inventory(10, 10),
            new Inventory(5, 10),
            new Inventory(1, 10)
        };
        Dictionary<int, int> result = new Dictionary<int, int>();

        DisperseNotes(amount, inventory.Select(x => x).ToList(), result);
        foreach (var item in result)
        {
            Console.WriteLine("Denomination: "   item.Key   ", Disbursed: "   item.Value   ", Available: "   (inventory.Where(x => x.Denomination == item.Key).Select(x => x.Quantity).First() - item.Value));
        }
    }

    private static void DisperseNotes(int amount, List<Inventory> inventory, Dictionary<int, int> result)
    {
        var inv = inventory.First();
        var val = amount / inv.Denomination;

        if (val > inv.Quantity)
            val = inv.Quantity;

        result.Add(inv.Denomination, val);
        amount = amount - (inv.Denomination * val);
        inventory.RemoveAt(0);

        if (inventory.Any())
            DisperseNotes(amount, inventory, result);
        else if (amount > 0)
            throw new Exception("Inventory insufficient to disperse amount.");
    }

    private class Inventory
    {
        public Inventory(int denomination, int quantity)
        {
            Denomination = denomination;
            Quantity = quantity;
        }

        public int Denomination { get; set; }
        public int Quantity { get; set; }
    }

CalculateDenominations method accepts the amount. I'm using a hardcoded inventory that will probably come from some database / system.

Code recursively goes through inventory trying to calculate denominations based on what's available in inventory.

I wrote that code as a console app so methods are static, but they don't really need to be static in actual code.

Code does use a bit of linq but this logic doesn't really require it.

Input:

CalculateDenominations(1122);

Result:

Denomination: 100, Disbursed: 10, Available: 0
Denomination: 20, Disbursed: 6, Available: 4
Denomination: 10, Disbursed: 0, Available: 10
Denomination: 5, Disbursed: 0, Available: 10
Denomination: 1, Disbursed: 2, Available: 8