Choose best function name in repository [duplicate]

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/336143

Question

In my code, I'm using repository pattern. For example, I got deal repository. I just curious how you decide the name of the function when the function itself doing so many things. I think when the function name too long it can decrease code readability. Is this considered bad because of function name too long in terms of clean code.

For example below is the list of the function I wrote in DealRepository Class:

  • GetAllDealsWithinValidRangeStartDateEndDateUserLimitAndQuota
  • GetAllDealsForCategoryWithinValidRangeStartDateEndDateUserLimitAndQuota
  • GetUniqueSubCategoriesForDealsWithinValidRangeStartDateEndDateUserLimitSubCategoryAndQuota
  • GetAllDealsForSubCategoryWithinValidRangeStartDateEndDateUserLimitAndQuota

The function name too long because when you want to retrieve deal, got many things to filter like deal within 10KM radius, deal start date, deal end date, deal quota, deal quota per user and others. Any suggestion or opinion how to choose a good name for this function.

Was it helpful?

Solution

The signature of the method is not only its name, but also the return type, the arguments, the generic parameters, etc. In your case, it looks like the names, for most part, repeat what is already specified by the arguments.

If your programming language supports overloading, you can end up with something like this:

  • DealRepository.GetAll(DateRange range, int userLimit, int quota)

  • DealRepository.GetAll(Category category, DateRange range, User user, int quota)

  • DealRepository.GetAll(SubCategory sub, DateRange range, User user, int quota)

  • DealRepository.GetSubCategories(DateRange range, User user, int quota)

By forcing the caller to specify the range, you show that the values you yield return will be within the range (otherwise, the method wouldn't make any sense.) The same goes for the user limit and the quota.

If your language has optional parameters too, you can reduce even more the number of methods:

  • DealRepository.GetAll(DateRange range, int userLimit, int quota, Category category: null)

  • DealRepository.GetAll(SubCategory sub, DateRange range, int userLimit, int quota)

  • DealRepository.GetSubCategories(DateRange range, int userLimit, int quota)

and then, by making Category and SubCategory inherit from the same abstract class Grouping, you can refactor that to:

  • DealRepository.GetAll(DateRange range, int userLimit, int quota, Grouping grouping: null)

  • DealRepository.GetSubCategories(DateRange range, int userLimit, int quota)

What happened with Unique?

By using the principle of least astonishment, you may indeed, like in my example below, omit Unique from the name. In other words:

  • If the user expects, by calling GetSubCategories, to receive only distinct values, don't add Unique to the method name.

  • If the user would assume that the method could yield duplicate values, then GetUniqueSubCategories is preferable to show that the values are necessarily unique. Or you may change the signature of the method to return a type which corresponds to a sequence containing only distinct values.

Does GetSubCategories really belong here?

If GetAll return a typed sequence (such as DealsCollection), you may move GetSubCategories to this class. Instead of doing:

new DealRepository().GetSubCategories(...)

the caller will rather do:

new DealRepository().GetAll(...).GetSubCategories()

A few remarks...

Aside the fact that WithinValidRangeStartDateEndDateUserLimitAndQuota has no place in the name of the method, there are a few issues as well:

  • WithinValidRange: why not simply WithinRange? Would it make sense for a caller to query for deals within an invalid range?

  • StartDateEndDate: you shouldn't do that even at object level. It's not Date start, Date end. It's a date range. So create a DateRange class.

  • All feels wrong. Currently, the caller doesn't retrieve all the deals, but only the deals which correspond to the specified filters.

  • Depending on the community using the language, it may be acceptable to remove Get prefix, leading to DealRepository.All(...) and DealsCollection.SubCategories().

  • If you are able to use the iterator pattern, then the method becomes:

    DealRepository.GetAll()

    without arguments. It is used this way:

    DealRepository
    .GetAll()
    .Where(d => range.Contains(d.date) && d.user == user)
    .Take(quota)

OTHER TIPS

It may be a good idea to have a single method that accepts an object as the parameter instead, if the queries are sufficiently configurable. This is called the Introduce Parameter Object refactoring.

public class GetDealsParameter
{
    public DateTime? Start { get; set; }
    public DateTime? End { get; set; }
    public int? Limit { get; set; }
    public string Category { get; set; }
}

public class DealRepository 
{
    public IEnumerable<Deal> GetDeals(GetDealsParameter dealsParam)
    {
        // ...
    }
}

In this example you would only set the values you would like to filter on.

If you need help constructing the parameters object you could also use the builder pattern.

Licensed under: CC-BY-SA with attribution
scroll top