I think money stuff is complicated and finicky enough to immediately make the example distractingly non-simple. In English, at least, taxes are usually charged and calculated on, not over things. Do you really mean transfer? Those often have fees rather than taxes. Or is it a Purchase? A Transaction? Isn't 'amount' clearer than 'transferValue?'. Is 'tax' supposed to be a percentage? A fraction? An absolute amount?
This sort of thing is probably why such examples are always blandly about cars and their colors and shapes and their areas.
I guess you mean typedefs and not separate classes for Money, TaxPercentage etc.
I recently came across a codebase that implement type classes for almost all types of things it handles and found that to be overkill. Almost always, name and age can be represented by a string and an integer respectively; separate Name and Age classes just reduce the readability in your code.
In a language that supports it reasonably, the advantage of genuinely separate types is that the language will prevent you from doing stupid things like accidentally multiplying to monetary amounts or adding a percentage to a monetary amount.
Reasonable support here mostly means that you can implement operator overloads, so that you can define e.g. a Money type that can be added but not multiplied.
Money handling may not actually be such a good example for this, because you should really have proper unit testing there anyway. I did work on a codebase that had types for SI units, and it was quite a nice experience.
Actually, I think using non standard types increases complexity, despite the more informative naming.
I would now have to look up the implementation of Money and TaxPercentage to be sure of what it does, whereas if you move the type names into the variable names like in the following examples: 'double total_money' and 'double tax_percentage' I would immediately understand what's going on.
I do see how a TaxPercentage type allows you to create a type that restricts itself to percentages. I feel that it is serves readability better to leave this checking to the function using the value itself rather than having the type do it.
Well I disagree, but I doubt I'll convince you if you prefer that style :). You're not really making best use of the type system. Here's some of the reasons I'd prefer modeling this kind of thing with types
How can you find all the uses of money throughout your codebase if they're just doubles? They'll be mixed up with all other uses of doubles.
What happens when you need to add precision because double is no longer good enough? Not only do you need to find them all but then factor them out to different types.
How do you support currencies? How do you know what currency "double total_money" is at the moment? The meaning is lost as soon as you pass it to another function. Even if you are only dealing with USD right now good luck adding currency support in the future if you're using primitives.
Yes, you might have to press a key shortcut to look at the definition to see this information, but duplicating it in the variable name everywhere you use it seems barely better than comments. Furthermore, the variable names can be wrong because they're not enforced by the typesystem. The typechecker can enforce for me that I don't pass a negative value or a value in the wrong currency. Variable names don't do any enforcement
Primitive obsession[0] also tends to lead to lots of duplication of logic. Multiple functions have tests that check what happens when passed an invalid value, logic that would be in one place with a type definition.
While not really related to the primitive obsession vs domain modelling debate - it's a really bad idea to use floating point numbers for monetary calculations, and from experience I can tell you how hard it is to fix this down the road in an existing codebase processing large amounts of money.
When I first started using Objective-C, I noticed how verbose the signatures were when passing messages, but really liked how it read when you were scanning the source.
After that, I stopped giving a shit about how long my identifier names are and just make sure they explain in a direct (but concise!) way what that method does or what that variable is.
The verbosity itself shouldn't be a problem, if it someway helps to understand the class / method meaning. But, on the other hand redundancy can be a problem in my opinion, specially considering there is always a context where your code lives.
I pushed about 6 PRs last week that did nothing but eliminate abbreviations. It was such a minor thing but made me feel so much better. A corollary to your rule is "don't make me think" when reading code. A reader should never have to stop to understand what code is doing, it should be clear. Abbreviations only buy you bytes in the source code at the expense of readability.
Often programmers instinctively name their code the same way they structure it: with as much elegance and brevity as possible.
If you think about it, elegance and brevity in naming serve no logical purpose. When reading names, understandability is the only goal; elegance and brevity do not assist in this goal. Thus, from a readability standpoint, it is better for names to be verbose and informative at the expense of elegance.
There are those out there that complain about how verbosity contributes to increased typing time and the probability of spelling errors, to which I have to say: Modern dev tools like Intellisense and autocomplete largely eliminate this type of error. Additionally, if you hate these modern tools, I still feel that, especially in extremely large and complex code bases, the benefits of writing clear and understandable code with verbose naming far outweigh the benefits one gains when using brevity in naming.
How would you use ints when there's division in the code? I've heard it's usually better to prefer ints to floating points, but in my limited experience I've found that it's easier said than done.
Either return cents (for decimal currencies), or more generally create a "currency" type with ints and conversion ratios for each "field" (for example a "US Dollar" type would have "dollars" and "cents", with ratios of "1" and "100". This means you can handle non-decimal currencies[1] like the Malagasy Ariary[2] and the Mauritanian Ouguiya[3] (1 ariary = 5 iraimbilanja, 1 ouguiya = 5 khoums)
As you said, easier said than done; but you really don't want FP errors popping up in financial calculations :-)
In this case, I'd argue that the names are still bad for a couple of reasons.
Firstly, it's too verbose - a nice middle ground would have been better: calculateTransferTax(Double transferValue, Double tax). It's fairly clear what the tax is being calculated over.
Secondly, I'd say that "tax" is a bad name for the tax percentage since it doesn't describe that what actually is (the percentage / rate of tax), and since it's a Double, I'd expect it to be "0.2" for a 20% tax rather than 20.0 which appears to be what the method would actually expect.
The me the best convention is Linux kernel coding style. I prefer slightly more verbose variable and function names on interpreted languages, to compensate the lack of static type checking, but calculateTaxOverTransfer() is probably too much.
Local variable names should be short, and to the point. If you have random integer loop counter, it should probably be called "i". Calling it "loop_counter" is non-productive, if there is no chance of it being mis-understood. Similarly, "tmp" can be just about any type of variable that is used to hold a temporary value.
If you are afraid to mix up your local variable names, you have another problem, which is called the function-growth-hormone-imbalance syndrome. See chapter 6 (Functions).
Longer names don't automatically make it easier to read. Quite the opposite.
In this particular example, probably the function should be eliminated completely. It's easier to read (value x tax / 100) than to hunt for the spec of calculateTaxOverTransfer() to see what it does. (I'm ignoring the weirdness of representing tax in % instead of a float; that's probably another opportunity for a refactor).
Even if there is reasonable business justification to have a specific function to calculate taxes over transfer (maybe multiple callers, the logic is more complex than shown, or there's expectation that the rule might change) something named calc_tax() doesn't reduce the least of your ability to understand what it does.
If there's business ambiguity (like many types of taxes), then yes, you may need to differentiate which tax you're calculating. Even then, I still prefer the C convention and having a single function that receives a parameter indicating the tax to calculate, like calc_tax( tax_type, amount, rate ).
But one could argue that it's a different use case altogether, and you're better off by refactoring your code, and encapsulating the logic in a class that understands different objects passed as parameters for a calc_tax method.
The two examples given in support of short local variables names rely heavily on convention or prior knowledge. I don't think they're strong arguments for making short variable names.
Naming the integer counter "i" is convention learned in CS class (initially) that has become common usage. If you have nested for loops then j, k are often conventional for naming the next counter variables, though without an enclosing "i" they would look strange to most people.
Likewise, tmp draws on existing knowledge of /tmp as the location for temporary data on Unix machines.
Also in many early programming languages, variables could be only one or two letters. And in Fortran, the variables i, j, k, l, m, n were implicitly integers and others were real (that itself was as you noted, a carryover from mathematical formula conventions).
Relying on convention is a good thing, as long as your team (and the larger community for that language) is mature enough to have a strong set of relatively easily discoverable conventions.
Strongly agree. Conventions can produce code that is easier to read and to write. Certainly to the uninitiated, most of the formulas in a math or physics textbook look completely opaque. But by following conventions on meaning of symbols, they are precise and expressive and also easier to write.
I'm not saying convention is bad, I don't think that at all. I'm saying that the two examples here aren't good advocates for a general principle of short local variable names. The examples work only because people already know what they mean, due to the pre-existing conventions.
When it comes to code readability, and ultimate reusability, I believe renaming variables is where most time refactoring is best spent. Not only does it encourage better naming, but if the naming or paradigm model is off, it encourages better design.
I'd vote for removing dead/useless code, reducing scope/visibility, making things immutable, and running an auto-formatter. In the case your code is being re-used by outsiders, in an API or something, yeah bad naming ends up being awful.
yes taking an extra 10 seconds to name a function in a more clear way could probably save hours of frantic reading and cross referencing for another engineer down the road, especially when fixing urgent bugs/issues
And please make sure the words are spelled correctly in your public API. A library we use at work has a typo in the word "Acknowledge" where it is "Acknowladge". Drives me up the wall every time I see it.
ya this drives me nuts. Especially when the "autocorrect" word checker inside the IDE complains about the typo and keeps showing a red squiggly line, for code that is in another library that I am not supposed to change within the project in the IDE. Drives me nuts
I agree with this post in the general sense, still I think you can go way overboard with this.
If you take as your goal this: "understanding what this piece of code does 1 year from now", as opposed to "I should name these variables and this function fully and correctly" then I think you'll do a lot better each time. There are numerous places where I can more quickly and easily consume "i" as apposed to "someArrayIndex", or "fname,lname" instead of "firstname, lastname", etc.
The article makes a clear case for keyword arguments, without mentioning it at all. In Smalltalk you would write something like:
calculateTax: percentage transferValue: value
This has the advantage over the proposed solution that where-ever you CALL such a method, the meaning of the passed-in arguments is clear ALSO in the calling context. Just having meaningful names for FORMAL arguments does not do anything about how the ACTUAL arguments are named, in the (often more than one) places where the method is called. Meaning, it is hard to know whether a given method-call has its arguments in correct order without a) Checking the method-definition for their expected order b) Checking from the calling context the meaning of the variables you pass in to the call. Do they agree? Takes some figuring out.
That requires of course that your language supports keyword arguments. In languages that don't you can simulate them by using a single object as the only argument. That may seem like extra work but it helps, especially when there are many "arguments". It also allows you to have default-arguments without requiring that they are at the end of the argument-list. I think this pattern is called "Argument Object".
40 comments
[ 2.6 ms ] story [ 79.4 ms ] threadThis would allow you to ensure only valid values are passed and would help avoid problematic floating point arithmetic :)
This sort of thing is probably why such examples are always blandly about cars and their colors and shapes and their areas.
I recently came across a codebase that implement type classes for almost all types of things it handles and found that to be overkill. Almost always, name and age can be represented by a string and an integer respectively; separate Name and Age classes just reduce the readability in your code.
Reasonable support here mostly means that you can implement operator overloads, so that you can define e.g. a Money type that can be added but not multiplied.
Money handling may not actually be such a good example for this, because you should really have proper unit testing there anyway. I did work on a codebase that had types for SI units, and it was quite a nice experience.
I would now have to look up the implementation of Money and TaxPercentage to be sure of what it does, whereas if you move the type names into the variable names like in the following examples: 'double total_money' and 'double tax_percentage' I would immediately understand what's going on.
I do see how a TaxPercentage type allows you to create a type that restricts itself to percentages. I feel that it is serves readability better to leave this checking to the function using the value itself rather than having the type do it.
How can you find all the uses of money throughout your codebase if they're just doubles? They'll be mixed up with all other uses of doubles.
What happens when you need to add precision because double is no longer good enough? Not only do you need to find them all but then factor them out to different types.
How do you support currencies? How do you know what currency "double total_money" is at the moment? The meaning is lost as soon as you pass it to another function. Even if you are only dealing with USD right now good luck adding currency support in the future if you're using primitives.
Yes, you might have to press a key shortcut to look at the definition to see this information, but duplicating it in the variable name everywhere you use it seems barely better than comments. Furthermore, the variable names can be wrong because they're not enforced by the typesystem. The typechecker can enforce for me that I don't pass a negative value or a value in the wrong currency. Variable names don't do any enforcement
Primitive obsession[0] also tends to lead to lots of duplication of logic. Multiple functions have tests that check what happens when passed an invalid value, logic that would be in one place with a type definition.
[0] http://c2.com/cgi/wiki?PrimitiveObsession
While not really related to the primitive obsession vs domain modelling debate - it's a really bad idea to use floating point numbers for monetary calculations, and from experience I can tell you how hard it is to fix this down the road in an existing codebase processing large amounts of money.
I am never sure how much to abbreviate a name, but I guess if it's not going to be used much it can be long without problems.
After that, I stopped giving a shit about how long my identifier names are and just make sure they explain in a direct (but concise!) way what that method does or what that variable is.
Looking forward to the follow-up post, "Choosing good types".
If you think about it, elegance and brevity in naming serve no logical purpose. When reading names, understandability is the only goal; elegance and brevity do not assist in this goal. Thus, from a readability standpoint, it is better for names to be verbose and informative at the expense of elegance.
There are those out there that complain about how verbosity contributes to increased typing time and the probability of spelling errors, to which I have to say: Modern dev tools like Intellisense and autocomplete largely eliminate this type of error. Additionally, if you hate these modern tools, I still feel that, especially in extremely large and complex code bases, the benefits of writing clear and understandable code with verbose naming far outweigh the benefits one gains when using brevity in naming.
Also, the "tax" argument is the tax _rate_. "tax" in this instance I would expect to mean the final amount. So I'd call that parameter "taxRate".
(And I'd use ints for everything, of course)
As you said, easier said than done; but you really don't want FP errors popping up in financial calculations :-)
[1]: https://en.wikipedia.org/wiki/Non-decimal_currency
[2]: https://en.wikipedia.org/wiki/Malagasy_ariary
[3]: https://en.wikipedia.org/wiki/Mauritanian_ouguiya
http://floating-point-gui.de/basic/
http://0.30000000000000004.com/
Firstly, it's too verbose - a nice middle ground would have been better: calculateTransferTax(Double transferValue, Double tax). It's fairly clear what the tax is being calculated over.
Secondly, I'd say that "tax" is a bad name for the tax percentage since it doesn't describe that what actually is (the percentage / rate of tax), and since it's a Double, I'd expect it to be "0.2" for a 20% tax rather than 20.0 which appears to be what the method would actually expect.
Local variable names should be short, and to the point. If you have random integer loop counter, it should probably be called "i". Calling it "loop_counter" is non-productive, if there is no chance of it being mis-understood. Similarly, "tmp" can be just about any type of variable that is used to hold a temporary value.
If you are afraid to mix up your local variable names, you have another problem, which is called the function-growth-hormone-imbalance syndrome. See chapter 6 (Functions).
Source: Linux kernel coding style - https://www.kernel.org/doc/Documentation/CodingStyle
In this particular example, probably the function should be eliminated completely. It's easier to read (value x tax / 100) than to hunt for the spec of calculateTaxOverTransfer() to see what it does. (I'm ignoring the weirdness of representing tax in % instead of a float; that's probably another opportunity for a refactor).
Even if there is reasonable business justification to have a specific function to calculate taxes over transfer (maybe multiple callers, the logic is more complex than shown, or there's expectation that the rule might change) something named calc_tax() doesn't reduce the least of your ability to understand what it does.
If there's business ambiguity (like many types of taxes), then yes, you may need to differentiate which tax you're calculating. Even then, I still prefer the C convention and having a single function that receives a parameter indicating the tax to calculate, like calc_tax( tax_type, amount, rate ).
But one could argue that it's a different use case altogether, and you're better off by refactoring your code, and encapsulating the logic in a class that understands different objects passed as parameters for a calc_tax method.
Naming the integer counter "i" is convention learned in CS class (initially) that has become common usage. If you have nested for loops then j, k are often conventional for naming the next counter variables, though without an enclosing "i" they would look strange to most people.
Likewise, tmp draws on existing knowledge of /tmp as the location for temporary data on Unix machines.
Edit: As are a vast majority of CS knowledge: set theory, calculus, matrix, and so on.
yes taking an extra 10 seconds to name a function in a more clear way could probably save hours of frantic reading and cross referencing for another engineer down the road, especially when fixing urgent bugs/issues
If you take as your goal this: "understanding what this piece of code does 1 year from now", as opposed to "I should name these variables and this function fully and correctly" then I think you'll do a lot better each time. There are numerous places where I can more quickly and easily consume "i" as apposed to "someArrayIndex", or "fname,lname" instead of "firstname, lastname", etc.
calculateTax: percentage transferValue: value
This has the advantage over the proposed solution that where-ever you CALL such a method, the meaning of the passed-in arguments is clear ALSO in the calling context. Just having meaningful names for FORMAL arguments does not do anything about how the ACTUAL arguments are named, in the (often more than one) places where the method is called. Meaning, it is hard to know whether a given method-call has its arguments in correct order without a) Checking the method-definition for their expected order b) Checking from the calling context the meaning of the variables you pass in to the call. Do they agree? Takes some figuring out.
That requires of course that your language supports keyword arguments. In languages that don't you can simulate them by using a single object as the only argument. That may seem like extra work but it helps, especially when there are many "arguments". It also allows you to have default-arguments without requiring that they are at the end of the argument-list. I think this pattern is called "Argument Object".