Search code examples
c#.netlinqrefactoring

refactor linq operations to avoid duplicate similar code


I'm trying to simplify this piece of code where the only difference is that, if the referenceDate.Hour it's greater than 20, we should only take items larger than today. Otherwise, we take larger or equal items.

using System;
using System.Collections.Generic;
using System.Linq;

public class Program
{
    public static void Main()
    {
        //todays date is 07/02
        //I have the daysToCut 10, 15, 20, 25
        //if my referenceDate.Hour is bigger than 8pm, than i want the next day to cut other than today
        //but if my referenceDate.Hour is less than 8pm, i want the next day to cut (regardless if it's today or not)
        
        int nextDayToTake = 0;
        var daysToCut = new List<int>{10, 15, 20, 25};
        var referenceDate = DateTime.Now; //output is 10 bc today is 07/02
        //var referenceDate = new DateTime(2023, 02, 10, 20, 00, 00); //output is 15 bc the date's day is 10 and hour equals 20pm
        
        if (referenceDate.Hour >= 20)
        {
            nextDayToTake = daysToCut.Any(x => x > referenceDate.Day) ? daysToCut.First(x => x > referenceDate.Day) : daysToCut.Min();
        }
        else
        {
            nextDayToTake = daysToCut.Any(x => x >= referenceDate.Day) ? daysToCut.First(x => x >= referenceDate.Day) : daysToCut.Min();
        }

        Console.WriteLine(nextDayToTake);
    }
}

I tried it with ternary operators only, but I still found it too complex to understand. Could you help me to refactor in a more elegant way?

var daysToCut = new List<int>{10, 15, 20, 25};
var referenceDate = DateTime.Now;
//var referenceDate = new DateTime(2023, 02, 10, 20, 00, 00);
        
var nextDayToTake = referenceDate.Hour >= 20 ? 
            daysToCut.Any(x => x > referenceDate.Day) ? daysToCut.First(x => x > referenceDate.Day) : daysToCut.Min() 
            : daysToCut.Any(x => x >= referenceDate.Day) ? daysToCut.First(x => x >= referenceDate.Day) : daysToCut.Min();

Console.WriteLine(nextDayToTake);

Solution

  • I would aim for isolating the most basic expressions that actually differ from each other inside of each if loop, and creating logic based on those expressions.

    Inside your if/else,

    if (referenceDate.Hour >= 20)
    {
        nextDayToTake = daysToCut.Any(x => x > referenceDate.Day)
            ? daysToCut.First(x => x > referenceDate.Day)
            : daysToCut.Min();
    }
    else
    {
        nextDayToTake = daysToCut.Any(x => x >= referenceDate.Day)
            ? daysToCut.First(x => x >= referenceDate.Day)
            : daysToCut.Min();
    }
    

    the differing expressions are the following emphasized ones:

    nextDayToTake = daysToCut.Any(x => x > referenceDate.Day)
           ? daysToCut.First(x => x > referenceDate.Day)
           : daysToCut.Min();

    nextDayToTake = daysToCut.Any(x => x >= referenceDate.Day)
           ? daysToCut.First(x => x >= referenceDate.Day)
           : daysToCut.Min();

    (The > and >= operators are the only actually differing parts in these expressions, but I will work with the whole expressions.)

    I will name these expressions predicates, seeing as predicate is an established name of the input parameter of the Enumerable.Any() overload you are using.

    To generalize a little, we could name the first predicate predicate A, and the second predicate predicate B:

    nextDayToTake = daysToCut.Any(predicate A)
           ? daysToCut.First(predicate A)
           : daysToCut.Min();

    nextDayToTake = daysToCut.Any(predicate B)
           ? daysToCut.First(predicate B)
           : daysToCut.Min();

    Now, predicate A and predicate B are both of the same data type; they are both a function that takes an int (x) as an input parameter and returns a bool value (based on x being compared to referenceDate.Day). The data type of both predicates is hence Func<int, bool>.

    The predicates can be defined as follows:

    Func<int, bool> predicateA = x => x > referenceDate.Day;
    Func<int, bool> predicateB = x => x >= referenceDate.Day;
    

    (For clarity, I'd prefer to name the input variable something more descriptive:)

    Func<int, bool> predicateA = dayInMonth => dayInMonth > referenceDate.Day;
    Func<int, bool> predicateB = dayInMonth => dayInMonth >= referenceDate.Day;
    

    Seeing as predicate A and predicate B share data type, we could create one single predicate variable and assign whichever of predicate A and predicate B is relevant. The single predicate would then be used in the exact same manner:

    nextDayToTake = daysToCut.Any(predicate)
           ? daysToCut.First(predicate)
           : daysToCut.Min();

    The condition for assigning the correct predicate is the one you have provided in your original code:

    Func<int, bool> predicate = referenceDate.Hour >= 20 ? predicateA : predicateB;
    

    (You would possibly want to create a helper variable, for readability:)

    var conditionForA = referenceDate.Hour >= 20;
    
    Func<int, bool> predicate = conditionForA ? predicateA : predicateB;
    

    Now, you can simply use your predicate in your calculation of nextDayToTake. No if loop or code duplication needed.

    var nextDayToTake = daysToCut.Any(predicate)
        ? daysToCut.First(predicate)
        : daysToCut.Min();
    

    Putting everything together, the implementation could look something along the lines of:

    var daysToCut = new List<int> { 10, 15, 20, 25 };
    
    var referenceDate = DateTime.Now;
    
    var conditionForA = referenceDate.Hour >= 20;
    
    Func<int, bool> predicateA = dayInMonth => dayInMonth > referenceDate.Day;
    Func<int, bool> predicateB = dayInMonth => dayInMonth >= referenceDate.Day;
    
    Func<int, bool> predicate = conditionForA ? predicateA : predicateB;
    
    var nextDayToTake = daysToCut.Any(predicate) ? daysToCut.First(predicate) : daysToCut.Min();
    

    Note:
    I would strongly suggest that you rename conditionForA, predicateA and predicateB to something more descriptive. I don't quite understand the logic in your code, so I have not made an attempt to create suitable variable names myself.


    Example fiddle here.