4 ms·
It is a shame that default arguments isn’t a bit longer. Perhaps it’s out of scope to talk about anti-patterns but in my experience default arguments cause a l
by gorgoiler 4y ago
It is a shame that default arguments isn’t a bit longer. Perhaps it’s out of scope to talk about anti-patterns but in my experience default arguments cause a lot of distress to a good code base.
Defaults are useful when you are providing a library function for other teams to use. If you’re inside a more private code base and doing work on the implementation of your team’s service then it is wise to avoid default arguments.
The problem is they provide a point after which it seems acceptable to add a flood of more default arguments. This is particularly the case for junior developers who lack confidence to refactor instead of patch. Default arguments go hand in hand with conditional logic and cause functions to bloat into do-everything multi-page monsters without any focus and no tractable flow of logic.
Forgive the contrived example, but what was once this:
def greet(name):
print(f”Hello {name}”)
ends up becoming this, all because no one would bite the bullet and pick this apart into individual functions:
def greet(
name,
language=None,
io=None,
is_ci=False,
and_return=False,
):
greeting = “Hello”
if language:
greeting = translate(greeting)
message = f”{greeting} {name}”
fn = print
flush = False
if is_ci:
fn = log
flush = True
fn(
greeting,
flush=flush,
io=io if io else stdout,
)
if and_return:
return greeting
The slow rot of more and more defaults makes the function longer and longer. Moreover, each time someone adds a new option it gets harder to justify why they shouldn’t do it when the previous person was allowed.
- kroolik 4y agoI think the example is a bit grey. In my opinion, function should list its dependencies and allow changing them. Having said that I dont believe the `is_ci` decision should happen in the function. The decision should happen at the entrypoint and it should drive which implementations the code will use for the dependencies. I would look for the reason of rot in making the function become the merge point of multiple context, not the default values per-se. Whether default arguments make merging multiple contexts in a single function easier - code reviews might help here. In any case, very good example
- rightbyte 4y agoYou don't suggest any solution. Do you want more function overloading or maybe config objects? Adding default parameters works well with existing code. It is not bad and lazy because it is easy.
- gorgoiler 4y agoImagine two different call sites want to do two different things with the message. One wants to log() it, another wants to print() it. In my example this has been implemented by passing a flag to greet() to tell it what to do. If greet() gave up responsibility for outputting the message and instead just constructed it, then your code would look like this: def site1(): print(greet(“x”)) def site2(): log(greet(“y”)) And greet would be half as long.
- Xelynega 4y agoExcept wouldn't the ideal in this case be to use default parameters? def site(message, port=print): port(greet(message))
- PoignardAzur 4y agoThis bit in particular: if and_return: return greeting Is a nice touch. I've definitely seen that pattern in the wild.
- davnn 4y agoI can't conceive where this would be useful, could you expand on this?
- lozenge 4y agoIt's more likely to be listDir(withSizes=False) I work on a very large codebase like this and most functions return lists of strings/tuples and most DTOs will be dictionaries with string keys. Instead of classes which have methods to retrieve information in different ways. Therefore parameters have been added to return information in more and more ways.
- goodoldneon 4y agoI usually see it for things like “return_generator”. Then you need to write an overloaded signature to show that it could return a list or generator depending on that param. Then it’s even worse when there’s also an “allow_raise” param, where the return type will not be None if allow_raise=True. Now you need to write 4 overloaded signatures to account for the 2 polymorphic params
- aldanor 4y agopandas.read_csv() says hi - https://pandas.pydata.org/docs/reference/api/pandas.read_csv.html https://pandas.pydata.org/docs/reference/api/pandas.read_csv...
- japanman425 4y agomatplotlib has entered the chat.
- nonethewiser 4y agoYeah pandas seems to flaunt a lot of convention about not grouping lots of different control flow into a single function. But at the same time I wonder how it would look refacotred. How many read from csv functions would we be left with?
- masklinn 4y ago> How many read from csv functions would we be left with? It probably couldn't be that, because many build on one another. Some are deprecated and others are clearly incompatible, but out of 50 parameters you likely could imagine calling this with 20 parameters if the environment and the CSV you're ingesting are wonky enough. I think feasible refactorings would be: - rationalise currently separate parameters into meatier objects e.g. there's at least half a dozen parameters which deal with dates parsing, a dozen which configure the low-level CSV parsing, etc... that could probably be coalesced into configuration objects - a builder-type API, but you'd end up at the same result using intermediate steps instead of a function, not really useful unless you leverage (1) and each builder step configures a non-trivial amount of the system, so rather than 50 parameters you'd have maybe 10 builder, each with 0~10 knobs - or you'd build the thing as a bunch of composable transformers on top of a base parser Of note: the latter at least might be undesirable from the Pandas POV, as it would imply layers of recursive Python calls, which might be much slower than whatever Pandas currently does (I've no idea).
- disgruntledphd2 4y agoI think that this style (such as it is) comes from R, and scientific computing more generally. I grew up with R and never realised how terrible long argument functions are until relatively recently.
- aerhardt 4y agoI think the example is indeed contrived - you said it yourself! To me it doesn't illustrate the problem with default parameters specifically. For example it shows that the programmer doesn't know dependency injection and first-class functions. Printing could be passed in as a function param, but then it might actually be sensible to provide a default callable (eg, print), depending on how the greet function is going to be called. Language seems to be a perfectly sensible thing to have a default on. Then, and_return... That is, like, super contrived, man... I mean if a programmer doesn't know that a function caller can simply call for side effects and ignore the return value, they likely have much bigger problems than their judgement to use defaults or not. I empathize with your plight though - you're probably a great programmer, and I think it's very difficult for someone who is good at their craft to come up with genuinely but subtly shitty examples.
- gorgoiler 4y agoYou are right — the return thing is very silly. What about this: def greet(…, bow=False): … if bow: take_a_bow() Except imagine take_a_bow() instead as 10 lines of code to perform the post-greeting bow-ceremony. That code takes additional arguments regarding what kind of hand flourish to perform while bowing. The flourish_type has to be an optional argument to greet (because bow is) but inside the bow you have to assert flourish_type is not None because you can’t bow without knowing what flourish to give. I’ve seen some dark stuff over the years. Also: Welcome to HN!
- m3047 4y agodef greet(bow=None): ... if bow: bow() def elsewhere(gesture): def bow(): print(gesture) greet(bow) Wrap that.
- rr808 4y agoOMG please leave your dependency injection in your Java project and keep Python clean with its easy to use list of default parameters.
- 4y ago
- Waterluvian 4y ago> Moreover, each time someone adds a new option it gets harder to justify why they shouldn’t do it when the previous person was allowed. I’m not sure languages should be limited in order to avoid problems with the lack of project leadership.
- petespeed 4y agoSRP violation?
- acc_297 4y agommmm yes the entire R ecosystem… Others also mention pandas Data science workflows in general love to do this