@ehmatthes this is pretty much the example I give for when I remove that extra variable.
When the function is named "get_something" and the last line is "return something", I'd rather remove the "something" variable when possible.
If the function name isn't the same as the variable name or the variable is assigned somewhere that isn't immediately before the last "return", I'd leave it.
@ehmatthes since I readability isn't an issue in this case (IMO) because the function name is the same as the variable name, the main reason I usually hear argued for keeping it is for the sake of ease of debugging.
@ehmatthes I'd prefer the right. But in a code review I'd only comment that the doc string should contain the raw value that's being parsed as an example 😁
@CodenameTim That's an interesting observation. I do that when the raw string isn't overly long.
But what if the raw value is really long and messy? I try to capture this kind of output, sanitize it, save it in a tests/resources/ directory, and write a unit test for the parsing function.
@ehmatthes I think having a realistic raw value in the tests is a great idea. However, having a shorter example of the format that's being parsed, even if it's extremely short is beneficial. If you have tests that confirm it works, then the doc string's purpose is to inform you what it does. Having to read the code to reverse engineer or switch to the tests to get an example is extra brain cycles.
I think I use the intermediate variable because it is more readable to me, but also because I so often end up going back to do a little more before the return statement. For example, deciding later to log that value.
@ehmatthes as usual, context dependent on how readable the 'final' expression is, and how likely it is that I might want to debug-log it at this point in the call stack sometime.
I normally /lean/ towards option 1, partly as I am also one of those filthy backwards <80char line length people 😂
@ehmatthes yep! Unless the operation is super simple like a single method call without arguments or maybe one argument. Think “strip()” or “reverse()” something very easy to grok.
@ehmatthes I prefer the intermediate variable for readability unless it's a one-liner.
Plus, in your example, someone could cause an IndexError and want to catch it and return None or an empty list(). I think it is more readable to handle the return value more easily than trying to catch an exception on a return statement. (personal nitpick)
@ehmatthes I prefer the more explicit name, but I definitely write the second way as well. I don't always think to add the descriptive variable name, but I appreciate it when I have to come back (or read some unfamiliar code).
@ehmatthes with a function this small I definitely prefer the second. I don't need the extra context from the name of the return value because it's in the function name. And I appreciate saving the vertical space.
Add comment