C# / .NET - operator AS performance/approach

176 Views Asked by At

Im working on a e-commerce sort of application.

Entities are:

  • Product with List<Expense> Expenses
  • Expense with Description props which is referring to packaging, transport, administrative costs etc.

I need to have at least 2 types of expenses:

  1. Relative expense (amount in percentage, calculated off the product price) and
  2. Absolute expense (amount in numbers)

What i tried doing is having an

  • abstract class Expense with props: Id, Description.
  • class RelativeExpense : Expense with AmountInPercentage props and
  • class AbsoluteExpense : Expensewith Amount props.

For calculating expenses I'm having a GetTotalExpenseAmount(Product p) method:

public decimal GetTotalExpenseAmount(Product p)

{
  decimal totalExpenses = 0;
  foreach (var expense in p.Expenses)
  {
       if(expense.GetType() == typeof(RelativeExpense))
       {
           totalExpenses += p.BasePrice * (expense as RelativeExpense).AmountInPercentage / 100;
       }
       else if(expense.GetType() == typeof(AbsoluteExpense))
       {
          totalExpenses += (expense as AbsoluteExpense).Amount;
       }
                
  }

    return totalExpenses;
}

My question is, is this a good practice? Reason i'm asking is because AS operator is doing casting and i know that can be expensive performance-wise. Plus i will have a Logger which will print out all expanses for a single product and therefore this foreach with as operator will be used again.

Is there any other way i can achieve what i want, but with better/cleaner code and performance? Maybe different modeling? Any ideas how should i approach this?

3

There are 3 best solutions below

0
On

I'd suggest a single class with IsRelative property:

class Expense 
{
    (...)
    public bool IsRelative {get;}
    public decimal AmountInPercentage {get;}
    public decimal Amount {get;
}
(...)
foreach (var expense in p.Expenses)
{
    if(expense.IsRelative)
    {
        totalExpenses += p.BasePrice * expense.AmountInPercentage / 100;
    }
    else
    {
        totalExpenses += (expense as AbsoluteExpense).Amount;
    }            
}

or even

class Expense 
{
    (...)
    public bool IsRelative {get;}
    public decimal Amount {get;}
}

(...)

foreach (var expense in p.Expenses)
{
    if(expense.IsRelative)
    {
        totalExpenses += p.BasePrice * expense.Amount / 100;
    }
    else
    {
        totalExpenses += expense.Amount;
    }            
}
8
On

Why casting can be expensive performance wise? I don't think so.

However, you can use the is operator here which also skips nulls:

foreach (var expense in p.Expenses)
{
   if(expense is RelativeExpense relativeExpense)
   {
       totalExpenses += p.BasePrice * relativeExpense.AmountInPercentage / 100;
   }
   else if(expense is AbsoluteExpense absoluteExpense)
   {
      totalExpenses += absoluteExpense.Amount;
   }         
}
4
On

You can uses the OOO Polymorphism. So the C# virtual/abstract methods.

public decimal GetTotalExpenseAmount(Product p)
{
  decimal totalExpenses = 0;
  foreach (var expense in p.Expenses)
  {
       totalExpenses += expense.GetPrice();
  }
  return totalExpenses;
}

public class Product {
    public string Name;
    public List<Expense> Expenses;
}

public abstract class Expense {
    public string Id;
    public string Description;
    
    public abstract int GetPrice();
}

public class RelativeExpense : Expense {
    public int AmountInPercentage;
    public int BasePrice;
    
    public override int GetPrice() {
        return BasePrice * AmountInPercentage / 100;
    }
}

public class AbsoluteExpense : Expense { 
    public int Amount;
    
    public override int GetPrice() {
        return Amount;
    }
}

One caveat I moved the BasePrice to RelativeExpense class.