Is it enough for methods to be distinguished just by argument name (not type) or is it better to name it more explicitly?

For example T Find<T>(int id) vs T FindById<T>(int id).

Is there any good reason to name it more explicitly (i.e. adding ById) vs keeping just argument name?

One reason I can think of is when signatures of the methods are the same but they have a different meaning.

FindByFirstName(string name) and FindByLastName(string name)

  • 4
    So when you overload Find to include T Find<T>(string name) or (int size) how do you plan to resolve the inevitable problems? – UKMonkey Nov 26 at 13:03
  • 2
    @UKMonkey what inevitable problems? – Konrad Nov 26 at 13:11
  • 3
    in the first case: if multiple entries have the same name then you would have to change the function signature; which means people will likely get confused with what it's meant to return; In the latter case, the argument is the same - and thus an illegal overload. You either start naming the function with "byX" or make an object for the argument so that you can have the equivalent of overload with same argument. Either works well for different situations. – UKMonkey Nov 26 at 13:15
  • 2
    @UKMonkey you can post an answer with some code examples if you want – Konrad Nov 26 at 13:18
  • 2
    Ids should probably be an opaque ID object and not just an int. In that way get compile-time checking that you do not use an id for an int or viceversa in some part of your code. And with that you can have find(int value) and find(ID id). – Bakuriu Nov 26 at 18:25
up vote 67 down vote accepted

Sure there is a good reason to name it more explicitly.

It's not primarily be the method definition that should be self-explanatory, but the method use. And while findById(string id) and find(string id) are both self-explanatory, there is a huge difference between findById("BOB") and find("BOB"). In the former case you know that the random literal is, in fact, an Id. In the latter case you're not sure - it might actually be a given name or something else entirely.

  • 9
    Except in the 99% of other cases where you have a variable or property name is a point of reference: findById(id) vs find(id). You can go either way on this. – Greg Burghardt Nov 26 at 12:39
  • 15
    @GregBurghardt: A particular value is not necessarily named the same way for a method and its caller. For example, consider double Divide(int numerator, int denominator) being used in a method: double murdersPerCapita = Divide(murderCount, citizenCount). You can't rely on two methods using the same variable name as there are plenty of cases where that is not the case (or when it is the case, it's bad naming) – Flater Nov 26 at 14:45
  • 1
    @Flater: Given the code in the question (finding stuff from some sort of persistent storage) I imagine you might call this method with a "murderedCitizenId" or "citizenId" ... I really don't think the argument or variable names are ambiguous here. And honestly I could go either way on this. It's actually a very opinionated question. – Greg Burghardt Nov 26 at 14:53
  • 4
    @GregBurghardt: You cannot distill a global rule from a single example. OP's question is in general, not specific to the example given. Yes, there are some cases where using the same name makes sense, but there are also cases where it doesn't. Hence this answer,, it's best to aim for consistency even if it's not necessary in a subset of use cases. – Flater Nov 26 at 14:57
  • 1
    Names parameters resolve the confusion after the confusion has already existed, as you need to look at the method definition, while explicitly named methods entirely avoid the confusion by having the name present in the method call. Also, you can't have two methods with the same name and argument type but different argument name, which means you're going to need explicit names in certain cases anyway. – Darkhogg Nov 26 at 18:19

There are reasons to prefer FindById (versus a compelling one in favor of Find: Brevity).

  1. Future-proofing: If you start with Find(int), and later have to add other methods (FindByName(string), FindByLegacyId(int), FindByCustomerId(int), FindByOrderId(int), etc), people like me tend to spend ages until they finally find the Find(int) method because they are looking for FindById(int). Not really a problem if you can and will change Find(int) to FindById(int) once it becomes necessary, but that is something that will maybe happen in the future - future proofing is all about these maybes.

  2. Make things easier to read. Find is perfectly fine if the call looks like record = Find(customerId); Yet FindById is slightly easier for reading if it's record = FindById(AFunction()); or record = FindById(AFunction() * A_MAGIC_NUMBER + AnotherFunction()); - although you could argue that you have bigger problems if you reach that point.

  3. Consistency. You can consistently apply the FindByX(int) / FindByY(int) pattern everywhere, but that won't work for Find(int X) / Find(int Y).

Personally, I prefer a different approach:

// Use Id<Customer> or CustomerID, depending on local coding standards
CustomerRecord Find(Id<Customer> id) 

Related: Strongly typing ID values in C#


The above is the answer to the question, the below is clarification and additional information based on feedback provided by comments.

The advantages of a simple Find all boil down to the initially mentioned Brevity, which is valued highly by some, and not so by others:

  • KISS. Find is simple and straightforward, and alongside operator[] it's one of the 2 most expected function names in this context. (Some popular alternatives being get, lookup, or fetch, depending on context). Some schools of thought oppose unconditionally applying the KISS principle.
  • As a rule of thumb, if you have a function name that is a single well-known word which accurately describes what the function does, you need a very compelling argument to prefer a function name made up from multiple words, even if the latter is slightly better at describing what the function does. See: Length vs NumberOfElements. What is and what isn't a "very compelling argument" is subjective.
  • Some schools of thought aim to avoid redundancy whereever possible. If we look at FindById(int id), we can easily remove redundancy by changing it to Find(int id). It's worth mentioning that some schools of thought disagree and do embrace redundancy under various circumstances.

Regarding the suggested alternative Find(Id<Customer> id), there was an upvoted comment that heavily criticizes it. I'm including the response in this answer because similar criticism did appear in the linked question as well, meaning they are shared by multiple readers.

  • Concern 1: It's an abuse of generics. A CustomerId and an OrderID are not the same thing, but the implementation ends up being similar. A common approach to similar implementations is metaprogramming. Using generics for metaprogramming is their purpose, not an abuse. There is value in a discussion about either exposing or hiding the generic, something which will ultimately come down to local coding standards.
  • Concern 2: It doesn't stop simple mistakes./It's a solution in search of a problem The linked version absolutely does stop plenty of mistakes, with the most common one being the wrong argument order in DoSomething(int customerId, int orderId, int productId). If it's used in green-field projects there is no need to expose the underlying implementation, thus preventing even more possible mistakes. Strongly typed Ids also resolve the issue the OP asked us about.
  • Concern 3: It really just obscures code. It's hard to tell that an id is held in int aVariable. It is easy to tell what kind of Id is held in Id<Customer> aVariable. That's because the latter is less obscure. Knowing what a variable represents is always more important than knowing how it does so, because unlike the "how", the "what" is useful on it's own.
  • Concern 4: These Ids are no strong types, just wrappers. std::string is just a wrapper around char*. If a class wraps something, it does not follow that the class isn't a strong type. Using a class to wrap the implementation detail of an integer id serves the principle of making interfaces easy to use correctly, hard to use incorrectly, by removing unnecessary and harmful functionality like bitwise operations. It also provides encapsulation, which makes it a lot easier to account for new requirements such as larger IDs, leading zeros, or alphanumeric IDs. On top of that, implementing CustomerId as a class supports the single responsibility principle.
  • Concern 5: It's over engineered. Here's a minimal version, although I do recommend adding operator== and operator!= as well, since I find they are easier to read than Equals.

.

public struct Id<T>: {
    private readonly int _value ;
    public Id(int value) { _value = value; }
    public static explicit operator int(Id<T> id) { return id._value; }
}
  • 10
    Why is brevity "extremely compelling"? Will you die younger if you type those extra four letters? – Eric Lippert Nov 26 at 15:00
  • 4
    @EricLippert: Each of the last few C# versions added a few features that don't add any new functionality, just improved code brevity. I imagine those were deemed a priority for the same reason. – BlueRaja - Danny Pflughoeft Nov 26 at 18:22
  • @jrh Unfortunately, I deal with a lot of "Why couldn't this be more vague?" In the project I'm currently working on, enumeration values and message names and some others are so long that you can only fit that on a line by itself (our style is the 80-char per line). If you need multiple in a statement, simple things end up as 5 or 10 lines. – Aaron Nov 26 at 20:41
  • 2
    @jrh I did not mean to imply that the very short names are acceptable; I generally do not appreciate abbreviated symbols. I meant that I actually do have to deal with FindByCustomerIDInUnitedStatesOnATuesdayWhenThePrinterIsOutOfInkAndWhen... often on my current project, as there are a lot of those. It's awful. Just offering up an anecdote where someone actually has to deal with that. I agree with your statements though. As long as you don't go crazy with it (like on my current project), sane maintainers are happy to have descriptive self-documenting symbols. – Aaron Nov 26 at 23:20
  • 1
    Generic typing here achieves very little beyond forcing you to define meaningless types just for the sake of supplying a type parameter - it doesn't stop you instantiating an Id<Customer> with the value of an Id<Employee>. Please avoid "cleverness" like this because it really just obscures code. If you really need an Id type (which you probably don't) then the only generic parameter it should ever have, if any, is the type of the underlying Id (i.e. string, int, guid...) – Ant P Nov 27 at 9:03

Another way of thinking about this is to use the type safety of the language.

You can implement a method such as:

Find(FirstName name);

Where FirstName is a simple object that wraps a string which contains the first name and means there can be no confusion as to what the method is doing, nor in the arguments with which it is called.

  • 4
    Not sure what your answer is to the OPs question. Do you recommend to use a name like "Find" by relying on the type of the argument? Or do you recommend to use such names only when there is an explicit type for the argument(s), and use a more explicit name like "FindById" elsewhere? Or do you recommend to introduce explicit types to make a name like "Find" more feasible? – Doc Brown Nov 26 at 12:45
  • 2
    @DocBrown I think the latter, and I like it. It's actually similar to Peter's answer, iiuc. The rationale as I understand it is two-fold: (1) It's clear from the argument type what the function does; (2) You cannot make mistakes like string name = "Smith"; findById(name);.which is possible if you use non-descript general types. – Peter A. Schneider Nov 26 at 13:34
  • 5
    While in general I am a fan of compile time type safety, be careful with wrapper types. Introducing wrapper classes for the sake of type safety can at times severely complicate your API if done in excess. e.g., the whole "artist formerly known as int" problem that winapi has; in the long run I would say most people just look at the endless DWORD LPCSTR etc. clones and think "it's an int / string / etc.", it gets to the point where you spend more time propping up your tools than you do actually designing code. – jrh Nov 26 at 14:34
  • 1
    @jrh True. My litmus test for introducing "nominal" types (differing only in name) is when common functions/methods don't make any sense in our use case, e.g. ints are often summed, multiplied, etc. which is meaningless for IDs; so I'd make ID distinct from int. This can simplify an API, by narrowing down what we can do given a value (e.g. if we have an ID, it will only work with find, not e.g. age or highscore). Conversely, if we find ourselves converting a lot, or writing the same function/method (e.g. find) for multiple types, that's a sign that our distinctions are too fine – Warbo Nov 26 at 19:20

I am surprised no one suggested to use a predicate such as the following:

User Find(Predicate<User> predicate)

With this approach not only you reduce the surface of your API but also give more control to the user using it.

If that isn't enough you can always expand it to your needs.

  • 1
    The problem is that it's less efficient due to the fact that it can't take advantage of things such as indices. – Solomon Ucko Dec 3 at 11:45

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.