aliihsan muhziroğlu
aliihsan muhziroğlu

Reputation: 23

How can i avoid looping through same data

TB_COST.Data is my DataTable, I am looping through data and calculating some fields. I want seperate functions for each calculation but i am looping the data repeatedly. I am looking for a way to avoid repeating.

I tried to loop the data once and call functions inside but i thought it will be harder to make bug fixing later.

    public void CalculateCost()//this function getting called at a button onclick
    {
        try
        {
            CalculateProductPrice();
            CalculateFreight();            
        }
        catch(Exception ex)
        {
            ShowMessageBox(ex.ToString());
        }
    }

    public void CalculateProductPrice()
    {
        decimal totalTonage = Convert.ToDecimal("0");
        decimal totalPrice = Convert.ToDecimal("0");
        decimal priceCarpiTonage = Convert.ToDecimal("0");
        for(int i = 0; i < TB_COST.Data.Rows.Count; i++)
        {
            decimal containerTonage = Convert.ToDecimal(TB_COST.Data.Rows[i]["CONTON"].ToString());
            totalTonage += containerTonage;
            decimal price = Convert.ToDecimal(TB_COST.Data.Rows[i]["PROPRI"].ToString());
            totalPrice += price;
            priceCarpiTonage +=  Convert.ToDecimal(containerTonage) * price; 
        }
        decimal productPrice = priceCarpiTonage/totalTonage;
        productPrice = (Math.Round(productPrice, 2, MidpointRounding.AwayFromZero));
        //ShowMessageBox(totalPrice + " " + priceCarpiTonage + " " + productPrice);
        T_PROP.Text = productPrice.ToString();
        T_TOTN.Text = totalTonage.ToString();                     
    }
    public void CalculateFreight()
    {
        decimal totalFreight = Convert.ToDecimal("0");
        for(int i = 0; i < TB_COST.Data.Rows.Count; i++)
        {
            decimal decimalFreightPrice = Convert.ToDecimal(TB_COST.Data.Rows[i]["CONPRI"].ToString());
            decimal containerTonnage = Convert.ToDecimal(TB_COST.Data.Rows[i]["CONTON"].ToString());
            decimal decimalFreightBoluContainerTonnage = decimalFreightPrice/containerTonnage;
            totalFreight += decimalFreightBoluContainerTonnage;                
        }
        //ShowMessageBox(totalFreight.ToString());
        totalFreight = (Math.Round(totalFreight, 2, MidpointRounding.AwayFromZero));                                  
        T_FREG.Text = totalFreight.ToString();
    }

Upvotes: 0

Views: 156

Answers (2)

Cleptus
Cleptus

Reputation: 3541

Based on your comments and on mr springers's answer I think you just need to add some helper functions and call them inside your for.

Note: To make it unit testable some info of what is TB_COST would be needed. Then the signature would be public CalculationsResult PerformCalculations(???? TB_COST) and it would not have the UI interaction.

public void PerformCalculations()//this function getting called at a button onclick
{
    try
    {
        CalculationsResult calculations = new CalculationsResult();

        for(int i = 0; i < TB_COST.Data.Rows.Count; i++)
        {
            DoMyCalculation(calculations, TB_COST.Data.Rows[i]);
            // Call other calculations
        }
        decimal productPrice = calculations.PriceCarpiTonage / calculations.TotalTonage;
        productPrice = (Math.Round(productPrice, 2, MidpointRounding.AwayFromZero));
        T_PROP.Text = productPrice.ToString();
        T_TOTN.Text = totalTonage.ToString();   


        totalFreight = (Math.Round(totalFreight, 2, MidpointRounding.AwayFromZero));                                  
        T_FREG.Text = totalFreight.ToString();         
    }
    catch(Exception ex)
    {
        ShowMessageBox(ex.ToString());
    }
}

private void DoMyCalculation(CalculationsResult calculations, DataRow row)
{
    calculations.TotalPrice += (decimal)row["MyColumn"];
}

public class CalculationsResult 
{
    public decimal TotalTonage { get; set; }
    public decimal TotalPrice { get; set; }
    public decimal PriceCarpiTonage { get; set; }
}

Upvotes: 1

Arthur S.
Arthur S.

Reputation: 482

The simplest way is to merge both methods to one big method:

After this, you have the problem that the method has become too large and it is not easy to maintain. So you have to refactor this method again.

You should now change this method to respect the solid principles. https://medium.com/@mirzafarrukh13/solid-design-principles-c-de157c500425

In the first step you should create a new object which represent one row of your data table. After that you should create a new method to load the data from the data table to a list of this object.

public class TbCost {
    public decimal Conton {get;set;}
    public decimal ProPri {get;set;}
    public decimal ConPri {get;set;}

    public decimal GetPriceCarpiTonage() {
        return Conton * ProPri;
    }

    public decimal GetFreightBoluContainerTonnage() {
        return ConPri / Conton;
    }
}

public void CalculateCost()//this function getting called at a button onclick
{
    try
    {
        decimal totalTonage = 0m;
        decimal totalPrice = 0m;
        decimal priceCarpiTonage = 0m;
        decimal totalFreight = 0m;

        IList<TbCost> tbCosts = ReadTbCosts(TB_COST.Data.Rows);

        foreach (TbCost tbCost in tbCosts)
        {
            totalTonage += tbCost.Conton;               
            totalPrice += tbCost.ProPri;
            priceCarpiTonage +=  tbCost.GetPriceCarpiTonage(); 
            totalFreight += tbCost.GetFreightBoluContainerTonnage();   
        }

        T_PROP.Text = Round(priceCarpiTonage/totalTonage);
        T_TOTN.Text = totalTonage.ToString();                                
        T_FREG.Text = Round(totalFreight);
    }
    catch(Exception ex)
    {
        ShowMessageBox(ex.ToString());
    }
}

// This method should be moved to a sperate class
private string Round(decimal decimalValue) {
    return (Math.Round(decimalValue, 2, MidpointRounding.AwayFromZero)).ToString();
}

Upvotes: 0

Related Questions