Combine two methods into one

How can I combine the methods below? and should i do it?

public IQueryable<ItemDTO> RepGetMonthItems(string inOut, string planFact, int month)
{
    return GetItemsWithCategory().
            Where(i => i.InOut.Equals(inOut)).
            Where(i => i.PlanFact.Equals(planFact)).
            Where(i => i.DateTime.Month.Equals(month));
}

public IQueryable<ItemDTO> RepGetYearItems(string inOut, string planFact, int year)
{
    return GetItemsWithCategory().
            Where(i => i.InOut.Equals(inOut)).
            Where(i => i.PlanFact.Equals(planFact)).
            Where(i => i.DateTime.Year.Equals(year));
}

      

+3


source to share


5 answers


I agree with the "do one thing" principle. But I also believe in DRY.

So extension methods to the rescue!

Ask the main function to do one thing (get the report items) and the extension methods by year or month.

Declare the class of the extension method as follows:

    public static class QueryExtensions
    {
        public static IQueryable<ItemDTO> ForYear(this IQueryable<ItemDTO> query, int year)
        {
            return query.Where(i => i.DateTime.Year.Equals(year));
        }

        public static IQueryable<ItemDTO> ForMonth(this IQueryable<ItemDTO> query, int month)
        {
            return query.Where(i => i.DateTime.Month.Equals(month));
        }
    }

      

Create a cut RepGetItems

method like this:



    public IQueryable<ItemDTO> RepGetItems(string inOut, string planFact)
    {
        return GetItemsWithCategory().
            Where(i => i.InOut.Equals(inOut)).
            Where(i => i.PlanFact.Equals(planFact));
    }

      

Usage looks like this:

 var yearResults = originalQuery.RepGetItems(input, fact).ForYear(2015);
 var monthResults = originalQuery.RepGetItems(input, fact).ForMonth(10);

      

or even:

 var yearMonthResults = originalQuery.RepGetItems(input, fact).ForYear(2015).ForMonth(10);

      

Complete flexibility, without losing the "single purpose" principle.

+2


source


No, not worth it. The three answers I see violate Robert's "Uncle Bob" Martin's "do one" rule. Read about it here .



+3


source


Perhaps you could use Nullable<int>

for year and month:

public IQueryable<ItemDTO> RepGetItems(string inOut, string planFact, int? year, int? month)
{
    return GetItemsWithCategory().
        Where(i => i.InOut.Equals(inOut)).
        Where(i => i.PlanFact.Equals(planFact)).
        Where(i => !year.HasValue  || i.DateTime.Year.Equals(year.Value)).
        Where(i => !month.HasValue || i.DateTime.Month.Equals(month.Value));
}

      

+1


source


Conditions .Where

can be easily added to the request.

public IQueryable<ItemDTO> RepGetYearItems(string inOut, string planFact, int monthyear, bool useMonth)
{
    var query = GetItemsWithCategory().
        Where(i => i.InOut.Equals(inOut)).
        Where(i => i.PlanFact.Equals(planFact));

    if (useMonth)
    {
        query = query.Where(i => i.DateTime.Month.Equals(monthyear));
    }
    else
    {
        query = query.Where(i => i.DateTime.Year.Equals(monthyear));
    }

    return query;
}

      

or using int?

public IQueryable<ItemDTO> RepGetYearItems(string inOut, string planFact, int? year, int? month)
{
    var query = GetItemsWithCategory().
        Where(i => i.InOut.Equals(inOut)).
        Where(i => i.PlanFact.Equals(planFact));

    if (year != null)
    {
        query = query.Where(i => i.DateTime.Year.Equals(year.Value));
    }

    if (month != null)
    {
        query = query.Where(i => i.DateTime.Month.Equals(month.Value));
    }

    return query;
}

      

Note that this version is different from @TimSchmelter's version ... It is not fixed (it depends on the SQL type), what happens if you have some "dead code" in your query (fragments of a query that is not "active "because some parameters have a specific meaning, for example, his request). His request clearly contains "dead code" (all i => !year.HasValue || i.DateTime.Year.Equals(year.Value)

). If possible, I prefer to skip it.

0


source


How about using an optional parameter here:

public IQueryable<ItemDTO> RepGetMonthItems(string inOut, string planFact, int month, int year = -1)
    {
        return GetItemsWithCategory().Where(i => i.InOut.Equals(inOut) 
          && i.PlanFact.Equals(planFact)) && year == -1? 
            i.DateTime.Month.Equals(month) : i.DateTime.Year.Equals(year));
    }

      

Take a look at this example:

enter image description here

As you can see, every time you call the Where () method, you are doing a lot of extra work.

0


source







All Articles