36

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 '18 at 13:03
  • 3
    @UKMonkey what inevitable problems? – Konrad Nov 26 '18 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 '18 at 13:15
  • 2
    @UKMonkey you can post an answer with some code examples if you want – Konrad Nov 26 '18 at 13:18
  • 3
    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 '18 at 18:25
69

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 '18 at 12:39
  • 16
    @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 '18 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 '18 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 '18 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 '18 at 18:19
37

Advantages of FindById().

  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 looking for FindById(int). Not really a problem if you can and will change Find(int) to FindById(int) once it becomes necessary - future proofing is about these ifs.

  2. 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());.

  3. Consistency. You can consistently apply the FindByX(int) / FindByY(int) pattern everywhere, but Find(int X) / Find(int Y) is not possible because they conflict.

Advantages of Find()

  • 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).
  • 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, use it. Even if there is a longer multi-word name that is slightly better at describing what the function does. Example: Length vs NumberOfElements. There is a tradeoff, and where to draw the line is subject of an ongoing debate.
  • It's generally good to avoid redundancy. If we look at FindById(int id), we can easily remove redundancy by changing it to Find(int id), but there is a trade off - we lose some clarity.

Alternatively you can get the advantages of both by using strongly typed Ids:

CustomerRecord Find(Id<Customer> id) 
// Or, depending on local coding standards
CustomerRecord Find(CustomerId id) 

Implementation of Id<>: Strongly typing ID values in C#

Comments here, as well as in the link above, raised multiple concerns regarding Id<Customer> that I would like to address:

  • Concern 1: It's an abuse of generics. CustomerId and OrderID are different types (customerId1 = customerId2; => good, customerId1 = orderId1; => bad), but their implementation is nearly identical, so we can implement them either with copy paste or with metaprogramming. While there is value in a discussion about either exposing or hiding the generic, metaprogramming is what generics are for.
  • Concern 2: It doesn't stop simple mistakes./It's a solution in search of a problem The main issue that's removed by using strongly typed Ids is the wrong argument order in a call to DoSomething(int customerId, int orderId, int productId). Strongly typed Ids also prevent other problems, including the one OP asked about.
  • Concern 3: It really just obscures code. It's hard to tell if an id is held in int aVariable. It's easy to tell that an Id is held in Id<Customer> aVariable, and we can even tell that it's a customer Id.
  • Concern 4: These Ids are no strong types, just wrappers. String is just a wrapper around byte[]. Wrapping, or encapsulation, is not in conflict with strong typing.
  • Concern 5: It's over engineered. Here's the minimal version, although I do recommend adding operator== and operator!= as well, if you don't want to rely exclusively on 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

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 '18 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 '18 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 '18 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 '18 at 19:20
1

I will vote for explicit declaration like FindByID.... Software should be built for Change. It should be open and closed (SOLID). So the class is open to add similar find method like let's say FindByName.. etc.

But FindByID is closed and its implementation is unit tested.

I won't suggest methods with predicates, those are good at generic level. What if based on the field(ByID) you have complete different methodology.

0

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 '18 at 11:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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