3 ms·

Yes, and a source file littered with tiny helper functions that do very specific things and don't make any sense except in the precise context in which they get

Yes, and a source file littered with tiny helper functions that do very specific things and don't make any sense except in the precise context in which they get called, isn't necessarily more readable. Here, "query_params" means "extract the last three query parameters, raw (i.e. not unescaped and not broken into key-value pairs)." The transformation shown makes precisely nothing more readable or easy to understand. "The second argument to map()" is just as easy for your brain to group into a black box to be analyzed later as a call to an opaque "query_params" function that you need to read the implementation of to really understand what the code is actually doing. Of course sometimes it's the best solution to just extract local helper functions, especially if the actual function just becomes too unwieldy and/or the helpers are called from more than one place, but in general I try to extract things that do something more general than the thing I'm extracting it from and have an interface / a purpose that's easy to understand and describe on its own. To stay with the example, actually extracting the query parameters would be a generic, extractable utility. Half-extracting the last three parameters because the function I'm writing needs precisely that for some reason, is a local helper function, and I'd only extract it if there's a good reason, certainly not to make an already trivial function no easier to read.

lbriner 4y ago
I wouldn't attack the example too much, it does seem a little contrived to make the point - the very thing I don't like about contriving examples! It is hard to know whether the principle is valuable with such a weird example. In this example there are lots of other ways it could have been done more meaningfully but, again, don't know if the example is real.
- mike_hock 4y ago
  What doesn't generalize is that the example doesn't necessarily stand for a simple one-liner, but as an example it has to be trivial, of course. But everything else generalizes. I took "splitting out the last 3 params" to mean "doing something very specific to the problem at hand" and extracting that isn't _always_ better.
overgard 4y ago
Your mileage clearly varies, but I found the transformed example much easier to understand. While I suspected it was parsing a query string from the initial code, having that stated explicitly in the variable removed the guessing. I think the main problem is he just didn't go far enough, there was still more to deconstruct. I suppose the function to parse the query string could have been better, its name isn't very descriptive, and the method with which it parsed wasn't very obvious either (I'd expect to get back a dict or a list of key/value tuples, not a list of strings) I know a lot of programmers are against comments, but I also think this is exactly the kind of code where a comment is handy..., the purpose of the [-3:] part wasn't obvious to me at all.
- mike_hock 4y ago
  It would presumably be evident from the context in which the `map` call appears what its purpose is.
  - overgard 4y ago
    And yet, I know python and have 15 years of professional experience coding, and (in the limited time I wanted to spend on it), I still don't exactly know what he was trying to accomplish. So congrats if you got it, but I'm sticking with my claim that it's fairly non-obvious. I understand that it's pulling out the last three elements. I have no idea why it's pulling out the last three elements, what purpose that serves and why it's desirable. If those elements were placed in a well named variable, I suspect it'd give a hint as to why the code is doing that. Alas.
    - mike_hock 4y ago
      That's precisely what we should expect from the context of the map call, but that context is outside the scope of the example.
- aidos 4y ago
  For me that's the issue with this whole example; it kinda misses the woods for the trees. # not interesting (also, use stdlib instead) def extract_url_params(url: str) -> dict[str, str]: params = url.split('?')[1].split('&') return = dict(param.split('=') for param in params) # This is the thing that needs explaining! final_3_param_values = list(extract_url_params(url).values())[-3:] By decomposing the problem in a more natural way, you can entirely direct the reader's attention to the thing that's causing the dissonance. For some reason it's just the the final 3 values they want (both weird requirements). Make that clear from the code!

lbriner 4y ago

I wouldn't attack the example too much, it does seem a little contrived to make the point - the very thing I don't like about contriving examples! It is hard to know whether the principle is valuable with such a weird example. In this example there are lots of other ways it could have been done more meaningfully but, again, don't know if the example is real.

mike_hock 4y ago
What doesn't generalize is that the example doesn't necessarily stand for a simple one-liner, but as an example it has to be trivial, of course. But everything else generalizes. I took "splitting out the last 3 params" to mean "doing something very specific to the problem at hand" and extracting that isn't _always_ better.

overgard 4y ago

Your mileage clearly varies, but I found the transformed example much easier to understand. While I suspected it was parsing a query string from the initial code, having that stated explicitly in the variable removed the guessing. I think the main problem is he just didn't go far enough, there was still more to deconstruct. I suppose the function to parse the query string could have been better, its name isn't very descriptive, and the method with which it parsed wasn't very obvious either (I'd expect to get back a dict or a list of key/value tuples, not a list of strings) I know a lot of programmers are against comments, but I also think this is exactly the kind of code where a comment is handy..., the purpose of the [-3:] part wasn't obvious to me at all.

mike_hock 4y ago
It would presumably be evident from the context in which the `map` call appears what its purpose is.
- overgard 4y ago
  And yet, I know python and have 15 years of professional experience coding, and (in the limited time I wanted to spend on it), I still don't exactly know what he was trying to accomplish. So congrats if you got it, but I'm sticking with my claim that it's fairly non-obvious. I understand that it's pulling out the last three elements. I have no idea why it's pulling out the last three elements, what purpose that serves and why it's desirable. If those elements were placed in a well named variable, I suspect it'd give a hint as to why the code is doing that. Alas.
  - mike_hock 4y ago
    That's precisely what we should expect from the context of the map call, but that context is outside the scope of the example.
aidos 4y ago
For me that's the issue with this whole example; it kinda misses the woods for the trees. # not interesting (also, use stdlib instead) def extract_url_params(url: str) -> dict[str, str]: params = url.split('?')[1].split('&') return = dict(param.split('=') for param in params) # This is the thing that needs explaining! final_3_param_values = list(extract_url_params(url).values())[-3:] By decomposing the problem in a more natural way, you can entirely direct the reader's attention to the thing that's causing the dissonance. For some reason it's just the the final 3 values they want (both weird requirements). Make that clear from the code!