6 ms·
While I certainly think that naming is important, I don't quite agree with all the improvements suggested here: > // Bad: > List<DateTime> holidayDateList; >
by zck 7y ago
While I certainly think that naming is important, I don't quite agree with all the improvements suggested here:
> // Bad:
> List<DateTime> holidayDateList;
> Map<Employee, Role> employeeRoleHashMap;
> // Better:
> List<DateTime> holidays;
> Map<Employee, Role> employeeRoles;
I'm not sure this is better. Yes, it's obvious that both are collections, but it's not obvious what you can do with them. Is `holidays` a map of date to information about the holiday? Is `employeeRoles` a set of all possible roles?
And once you remember that `employeeRoles` is a map, you're still a little lost about how to look into it. Is the key the employee id? Email?
Perhaps it could be called employeeToRoleMap? That distinguishes it from employeeIdToRoleMap, or possibleEmployeeRoles.
Maybe this is me disagreeing with a later statement by the author:
> Some people tend to cram everything they know about something into its name. Remember, the name is an identifier: it points you to where it’s defined. It’s not an exhaustive catalog of everything the reader could want to know about the object. The definition does that. The name just gets them there.
I don't know if I want to know everything, but I think I lean towards more than the author does. A variable name I changed today was originally called `initial-points`. It designated the set of intermediate points directly from a source to a destination, which would later be offset randomly (example: https://imgur.com/a/UxkGPfX https://imgur.com/a/UxkGPfX).
While refactoring related code, I realized that although it was descriptive, it was descriptive _temporally_, rather than describing the identity of the points. I ended up changing it to `points-in-direct-line`. It still feels subpar to me, but at least it's clearer: the points exist not because they'll be changed later (initial-points), but as a direct line between the source and destination.
- joshuamorton 7y agoIn a strongly typed language, you shouldn't encode type information in the name, it's statically derivable. Holidays isn't a map, it's a list. The name doesn't need to tell you that, you already know and the language will prevent you from misusing it. With employees, you might be right. The comments on the original article give two suggestions: 1. `employeesById`. This may imply a mapping type, but you aren't stuttering (saying map twice in the decl) at least. 2. Create an employee ID type and make the type declaration Map<EmployeeId, Employee>. (Where your id type might just bsubclass int) This way the semantic information is encoded into the type, and the type system prevents your from misusing things. For example you'd need to explicitly cast from int to employee ID.
- zck 7y ago> In a strongly typed language, you shouldn't encode type information in the name, it's statically derivable. This is completely true! > The name doesn't need to tell you that, you already know and the language will prevent you from misusing it. I think this is where we differ! I prefer to know what I can do with something without having to ask a compiler or IDE. This helps, for example, when looking at a pull request -- you don't have the code easily accessible for the compiler or IDE. Possibly a difference is that I prefer to use languages that are more dynamically typed -- Clojure, Emacs Lisp, etc. And so you don't have `Map<EmployeeId, Employee> employeesById`; you only have the variable name. But I do wonder -- even in the most explicitly typed, statically typed languages, what is the eser experience for finding this out? It seems that unless one is looking at the declaration, there must be at least one level of indirection to find out what the type of something is. Say you're looking at a use of the variable `employees`. Here's the ways I can think of that would let you know what the type is: 1. Scan upwards until you find the declaration, if it even is on screen. Then look back to where the use was. 2. Move the cursor to the variable, and use the "go to definition" functionality built into the IDE. Then look at the declaration, and use the "go back" IDE function. 3. Move the cursor to the variable, and the IDE has somewhere that tells you the type. This is relatively simple, but still requires you to move to the variable, and to look somewhere else and back. On the other hand, with a name like `employeesById`, all you need is in that thirteen characters.
- JadeNB 7y ago> I think this is where we differ! I prefer to know what I can do with something without having to ask a compiler or IDE. This helps, for example, when looking at a pull request -- you don't have the code easily accessible for the compiler or IDE. I think, in accordance with "variables won't and constants aren't", I would propose the less pithy: "any invariant that is not enforced is broken". The compiler doesn't check that the capabilities or roles advertised by your variable names are actually present, so eventually they won't be. That means that you can't just check the variable name when reviewing code, must must check the declaration (as well as possibly elsewhere) in case the variable names lies; and, once you're checking that, what have you gained in reviewability?
- kortex 7y agoMostly agree, but I still think we should be using shorter variable names and put all that information and context elsewhere. Opinion: It's (nigh) 2020. If you aren't using an editor which supports inspection and jump-to-declaration, you are doing it wrong. Similarly, if you are using a dynamic language and not leveraging type hinting or similar, you are doing it wrong. Where "doing it wrong" == doing a disservice to yourself, your coworkers, and future you. We have so much powerful metadata available right now. So with the examples above, you should be able to focus on `employeeRoles`, hit `hotkey`, and see a gist of the types, key format, what to do with that thing, and why it's there. Names should be unique and memorable enough that once you form that mental schema, you can quickly scan through code and understand its role in the greater machinations, without too much mental re-parsing.
- zck 7y agoThe benefit of naming things to explain them is to obviate the need for jump-to-declaration. Yes, any time I need to know what 5*6 is, I can walk into the other room and get my calculator -- but if I have that knowledge in my head, I don't need to go look it up. I think it's relatively agreed-upon that single-character variable names are bad, because they don't tell us enough about what the variable holds. So, too, do I feel about the type, when it's not obvious from context. So `age` would be clear enough that it's numeric^1. But what about the other example of `employees`? I would not know what I can do with that without having to jump back to the definition! Naming it `employeesById` tells you what you can do with it. And that seems like a win to me. [1] It might not differentiate between integers and floats, but that feels too nitpicky to me to put in the name.
- leetcrew 7y agoas someone who works on a large c++ project, I disagree, mainly just because it can take a long time for intellisense to actually jump me to the definition. there are also many ways intellisense can get confused and present me with a page of 100 similar function definitions that I need to scroll through. few things break my concentration like waiting 10+ seconds for intellisense to decide whether it's actually going to be able to jump me to the definition. imo, functions and variables should be named in such a way that the reader can get a pretty good idea of what's going on without jumping into a bunch of other scopes. try not to make them too long, but err on the side of too long rather than too short.
- resource0x 7y agoMap<Employee, Role> employeeToRole?
- layer8 7y agoI often use the name pattern `rolesByEmployee` in that situation.