Checking overlap ranges between two prices

Question

I have a method (C#, .NET framework 4.0) to check overlapping ranges between two prices.

• I have a process, which will have one or more price(s).
• A price has one or more range(s), like height, length, width, etc.
• Each range has a min and max value.

How can I further optimize this code? Or is there a better way of doing this?

    public static bool CheckOverlapPriceRange(int formulaId, int rangeInfoId, ref List<int> overlapRangeInfoIds)
    {
        //TO CHECK IF RANGES(RANGE ITEMS) OF RANGE INFO IS/ARE OVERLAP WITH OTHER RANGE INFO RANGES UNDER SAME FORMULA.
        List<DAL.SurfaceTreatment.vw_ST_RangeItem_RangeInfo_Formula> rangeItemInfoFormulaToCompare = DAL.RangeInfoHandler.GetInvertLockedVwRangeItemRangeInfoFormula(formulaId, rangeInfoId);

        if (rangeItemInfoFormulaToCompare.Count > 0)
        {
            List<DAL.SurfaceTreatment.vw_ST_RangeItem_RangeInfo_Formula> rangeItemInfoFormulaToCheck = DAL.RangeInfoHandler.GetVwRangeItemRangeInfoFormula(formulaId, rangeInfoId);
            //bool overlap = true;
            //bool overlapInfoId = false;
            //BOOL OF ITEM IN EACH RANGE INFO.
            List<bool> overlapBools = new List<bool>();

            //LIST TO STORE PAIR OF RANGE INFO. ID AND BOOL TO DECIDE IF OVERLAP
            List<Tuple<int, bool>> rangeInfoIdBool = new List<Tuple<int, bool>>();

            //BOOL OF EACH RANGE INFO.
            //List<bool> overlapRangeInfoId = new List<bool>();

            //GET DISTINCT RANGE INFO. ID FROM FORMULAS TO COMPARE
            var infoIdsToCompare = rangeItemInfoFormulaToCompare
                    .Select(x => x.FK_rangeInfoId)
                    .Distinct()
                    .OrderBy(x => x);

            //*******************************************************************************
            //START - ONLY CHECK IF BOTH PRICES/RANGE INFOS HAVE THE SAME RANGES/MEASUREMENTS
            //*******************************************************************************
            List<DAL.SurfaceTreatment.vw_ST_RangeItem> rangeItemsToCheck = DAL.RangeItemHandler.GetVwRangeItems(rangeInfoId);

            foreach (int infoIdToCompare in infoIdsToCompare)
            {
                List<DAL.SurfaceTreatment.vw_ST_RangeItem> rangeItemsToCompare = DAL.RangeItemHandler.GetVwRangeItems(infoIdToCompare);

                //IF BOTH PRICES/RANGE INFOS HAVE SAME RANGE ITEMS/MEASUREMENT
                if (IsSameRangesMeasurement(rangeItemsToCheck, rangeItemsToCompare))
                {
                    foreach (DAL.SurfaceTreatment.vw_ST_RangeItem_RangeInfo_Formula tempToCheck in rangeItemInfoFormulaToCheck)
                    {
                        //FILTER FORMULAS TO COMPARE BY MEASUREMENT AND RANGE INFO. ID
                        List<DAL.SurfaceTreatment.vw_ST_RangeItem_RangeInfo_Formula> listFilterByMeasurementInfoId = rangeItemInfoFormulaToCompare
                            .Where(x => string.Compare(x.measurementName.Trim(), tempToCheck.measurementName.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
                            //.Where(x => x.FK_rangeInfoId == infoId)
                            .ToList();

                        foreach (DAL.SurfaceTreatment.vw_ST_RangeItem_RangeInfo_Formula tempToCompare in listFilterByMeasurementInfoId)
                        {
                            bool conditionA = false;
                            bool conditionB = false;

                            //IF BOTH ARE SAME MEASUREMENT
                            if (tempToCheck.measurementName.Trim().ToLower() == tempToCompare.measurementName.Trim().ToLower())
                            {
                                #region ASSUME ONE RANGES ARE BEFORE ANOTHER RANGES
                                //IF EITHER TO CHECK MIN OR TO COMPARE MAX IS INCLUSIVE
                                if (tempToCheck.rangeItemMinInclusive || tempToCompare.rangeItemMaxInclusive)
                                {
                                    //IF TO CHECK MIN EQUAL TO TO COMPARE MAX
                                    if (tempToCheck.rangeItemMin == tempToCompare.rangeItemMax)
                                    {
                                        //IF TO CHECK MIN AND TO COMPARE MAX ARE INCLUSIVE
                                        if (tempToCheck.rangeItemMinInclusive && tempToCompare.rangeItemMaxInclusive)
                                        {
                                            //THEY ARE OVERLAP
                                            //EXAMPLE
                                            //0 < X <= 1
                                            //         1 <= X < 2 
                                            overlapBools.Add(true);
                                            rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, true));
                                            break;
                                            //return true;
                                        }
                                        else
                                        {
                                            conditionA = (tempToCheck.rangeItemMin >= tempToCompare.rangeItemMax);
                                        }
                                    }
                                    else
                                    {
                                        conditionA = (tempToCheck.rangeItemMin >= tempToCompare.rangeItemMax);
                                    }
                                }
                                //IF NEITHER TO CHECK MIN OR TO COMPARE MAX IS INCLUSIVE
                                else if (!tempToCheck.rangeItemMinInclusive && !tempToCompare.rangeItemMaxInclusive)
                                {
                                    //IF MIN EQUAL TO MAX
                                    if (tempToCheck.rangeItemMin == tempToCompare.rangeItemMax)
                                    {
                                        //THEY ARE NOT OVERLAP
                                        //EXAMPLE
                                        //0 < X < 1
                                        //        1 < X < 2
                                        overlapBools.Add(false);
                                        rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, false));
                                        break;
                                    }
                                    else
                                    {
                                        conditionA = (tempToCheck.rangeItemMin > tempToCompare.rangeItemMax);
                                    }
                                }
                                else
                                {
                                    conditionA = (tempToCheck.rangeItemMin > tempToCompare.rangeItemMax);
                                }
                                #endregion

                                #region ASSUME ONE RANGES ARE AFTER ANOTHER RANGES
                                //IF EITHER TO CHECK MAX OR TO COMPARE MIN IS INCLUSIVE
                                if (tempToCheck.rangeItemMaxInclusive || tempToCompare.rangeItemMinInclusive)
                                {
                                    //IF TO CHECK MAX EQUAL TO TO COMPARE MIN
                                    if (tempToCheck.rangeItemMax == tempToCompare.rangeItemMin)
                                    {
                                        //IF BOTH TO CHECK MAX AND TO COMPARE MIN ARE INCLUSIVE
                                        if (tempToCheck.rangeItemMaxInclusive && tempToCompare.rangeItemMinInclusive)
                                        {
                                            //THEY ARE OVER LAP
                                            //EXAMPLE
                                            //0 < X <= 1
                                            //         1 <= X < 2
                                            overlapBools.Add(true);
                                            rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, true));
                                            break;
                                            //return true;
                                        }
                                        else
                                        {
                                            conditionB = (tempToCheck.rangeItemMax <= tempToCompare.rangeItemMin);
                                        }
                                    }
                                    else
                                    {
                                        conditionB = (tempToCheck.rangeItemMax <= tempToCompare.rangeItemMin);
                                    }
                                }
                                //IF NEITHER TO CHECK MAX OR TO COMPARE MIN ARE INCLUSIVE
                                else if (!tempToCheck.rangeItemMaxInclusive || !tempToCompare.rangeItemMinInclusive)
                                {
                                    //IF TO CHECK MAX EQUAL TO TO COMPARE MIN
                                    if (tempToCheck.rangeItemMax == tempToCompare.rangeItemMin)
                                    {
                                        //THEY ARE NOT OVERLAP
                                        //EXAMPLE
                                        //0 < X < 1
                                        //        1 < X < 2
                                        overlapBools.Add(false);
                                        rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, false));
                                        break;
                                    }
                                    else
                                    {
                                        conditionB = (tempToCheck.rangeItemMax < tempToCompare.rangeItemMin);
                                    }
                                }
                                else
                                {
                                    conditionB = (tempToCheck.rangeItemMax < tempToCompare.rangeItemMin);
                                }
                                #endregion

                                //IF EITHER CONDITION IS TRUE, THEY ARE NOT OVERLAP
                                if (conditionA || conditionB)
                                {
                                    overlapBools.Add(false);
                                    rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, false));
                                }
                                else
                                {
                                    overlapBools.Add(true);
                                    rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, true));
                                }
                            }
                        }
                    }
                }
                else
                {
                    //THIS infoIdToCompare IS NOT OVERLAP, HENCE SET TO FALSE
                    rangeInfoIdBool.Add(new Tuple<int, bool>(infoIdToCompare, false));
                }
            }

            if (rangeInfoIdBool.Count > 0)
            {
                rangeInfoIdBool = rangeInfoIdBool.OrderBy(x => x.Item1).OrderBy(x => x.Item2).ToList();

                bool isOverlap = true;
                foreach (int infoId in infoIdsToCompare)
                {
                    isOverlap = true;
                    var filterList = rangeInfoIdBool.Where(x => x.Item1 == infoId).ToList();
                    foreach (var tempFilter in filterList)
                    {
                        isOverlap = isOverlap && tempFilter.Item2;
                    }

                    //AS LONG AS THERE IS OVERLAP WITH ANY ONE OF THE RANGE INFO.
                    if (isOverlap)
                    {
                        overlapRangeInfoIds = filterList.Select(x => x.Item1).Distinct().ToList();

                        //foreach(var overlapRangeInfoId in overlapRangeInfoIds)
                        //{
                        //    if (CheckOverlapCategoryBetweenRangeInfos(rangeInfoId, overlapRangeInfoId))
                        //    {
                        //    }

                        //}

                        return isOverlap;
                    }
                }
                return isOverlap;
            }
            else
            {
                return false;
            }
            //if (overlapBools.Count > 0)
            //{
            //    foreach (bool tempBool in overlapBools)
            //    {
            //        overlap = (overlap && tempBool);
            //    }
            //    return overlap;
            //}
            //else
            //{
            //    return overlap;
            //}
        }
        else
        {
            //IF ONLY ONE PRICE/RANGE INFO, THERE IS NO OVERLAP TO CHECK
            //HENCE RETURN FALSE
            return false;
        }
    }

Answer 1

I am adding my comments at first glance. This is not a final review as this code needs lots of refactoring.

1. Delete commented code

   Never keep commented code; it adds distraction while reading by anyone else.

2. Always check negative logic first to avoid nesting

   Check conditional logic first to avoid nested statements as mentioned below rather returning at last. It will add readability in your code.

   if (rangeItemInfoFormulaToCompare.Count==0)
       return false;
   

3. Unnecessary object creation

   I did not find any usage to this overlapBools.

4. Break your methods and condition check into smaller methods

   To avoid clutter in your code, always check and calculate the complex statement into a method. You can later merge it back into the main method if it seems unnecessary.

5. Proper naming convention

   temptocheck, ConditionA and ConditionB will not help you much when you will require the read this code again. Please correct this.

6. Merge code as needed

   if (conditionA || conditionB)
   {
       rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, false));
   }
   else
   {
       rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, true));
   }
   
   //This code can be rewritten as this, 
   rangeInfoIdBool.Add(new Tuple<int, bool>(tempToCompare.FK_rangeInfoId, 
   !(conditionA || conditionB)));
   

7. Try to encapsulate your condition logic into a class which handles your complex logic in one place.

   public class RangeComparer
   {
       private readonly Range _source;
       private readonly Range _target;
   
   
       public RangeComparer(Range source, Range target)
       {
           _source = source;
           _target = target;
       }
   
       public bool Compare()
       {
           //TODO Please include your logice her
       }
   }
   

8. Unit testing

   Please write tests so that these problems could be avoided easily.

Answer 2

You should start with making this code more readable by

• reducing the horizontal spacing by introducing a guard clause like for instance

  if (rangeItemInfoFormulaToCompare.Count == 0) { return false; }  
  

  could be placed at the top instead of if (variable > 0) {do something } else {return false}

• adding more usages of var where the type is obvious form the right side of the assignment.
• deleting commented out code, because such code is dead code and should be deleted.
• reducing the amount of comments. Comments are for telling why something is done. What is done should be told by the code itself by using meaningful names for variables, methods and classes.Btw, SHOUTING in comments isn't the way to go.

---

if (IsSameRangesMeasurement(rangeItemsToCheck, rangeItemsToCompare))
{
    foreach (var tempToCheck in rangeItemInfoFormulaToCheck)
    {
        //FILTER FORMULAS TO COMPARE BY MEASUREMENT AND RANGE INFO. ID
        var listFilterByMeasurementInfoId = rangeItemInfoFormulaToCompare
            .Where(x => string.Compare(x.measurementName.Trim(), tempToCheck.measurementName.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
            .ToList();

        foreach (var tempToCompare in listFilterByMeasurementInfoId)
        {
            bool conditionA = false;
            bool conditionB = false;

            //IF BOTH ARE SAME MEASUREMENT
            if (tempToCheck.measurementName.Trim().ToLower() == tempToCompare.measurementName.Trim().ToLower())
            {

Here you are creating a List<T> by using linq with a condition on the measurementName's being the same, which is done in a good way using string.Compare with a StringComparision enum but a few lines later you are again checking if both names are the same (although in a sub optimal way).
This second check should be skipped if you don't want to distinguish between culture relevant naming differences.

The call to ToList() on the resulting IEnumerable<T> of the mentioned linq query isn't needed at all, because you are only iterating over it.