> On Mar 15, 2020, at 7:42 PM, tyson andre <tysonandre...@hotmail.com> wrote: > > Hi Mike, > >> What I am about to say is ironic since I too often feel people are trying to >> enforce their view of coding on the world, >> but I do not think we want to give people tools to make it easier to work >> with long functions/scripts. > > Blocked-scoped variables is a tool which can make short code and long code > more readable. > Making it easier to work with long functions is an example of a benefit of > having block-scoped variables, > not the main goal of this RFC idea. > > https://github.com/getify/You-Dont-Know-JS/blob/1st-ed/scope%20%26%20closures/ch3.md#blocks-as-scopes > goes into some of the other reasons to use block-scoped variables in JS > > - Many languages other than JavaScript/PHP support Block Scope, > and so developers from those languages are accustomed to the mindset, > whereas those who've primarily only worked in JavaScript may find the concept > slightly foreign. > - Block scope is a tool to extend the earlier "Principle of Least Privilege > Exposure" > from hiding information in functions to hiding information in blocks of our > code. > - Developers may prefer to check themselves against accidentally (re)using > variables outside of their intended purpose, > such as being issued an error about an unknown variable if you try to use it > in the wrong place. > Block-scoping (if it were possible) for the `i` variable would make `i` > available only for the for-loop, > causing an error if `i` is accessed elsewhere in the function. > This helps ensure variables are not re-used in confusing or hard-to-maintain > ways.
I hear you. Personally, I do not find any of those compelling enough to outweigh the concerns I have already raised. But again, it is just my opinion and I respect your right and anyone else's to evaluate the pros and cons differently than I do. > >> I would be very interested in seeing business logic coded into a long >> function that literally cannot be made better by refactoring into multiple >> smaller functions. >> If you have some, please do post to a Gist and share it to see if it in fact >> it cannot be refactored and made "better." > > One example is if PHP itself is used for HTML rendering in top-level > statements - pages or components of a page may be spread out across hundreds > of lines. > This could be moved to a different templating engine for PHP instead of raw > PHP, > but the refactoring would be time-consuming and error-prone. Since you did not provide any specific examples I will overlook this... > Separately from HTML rendering, some problem domains are just complex. > For example, consider the functions such as decodeMPEGaudioHeader in > https://github.com/WordPress/WordPress/blob/master/wp-includes/ID3/module.audio.mp3.php > (I'm not involved in that project or file) Ironically, I work primarily with WordPress, and while I do so I find much of the code in WordPress core to be bordering on completely awful, and this code is no exception. Looking at the decodeMPEGaudioHeader() function it is over 600 lines of meandering spaghetti, and a wonderful example of why enabling people to make these kind of functions more maintainable is akin to giving addicts the strongest, most addictive drugs imaginable. :-) It would take more than 30 minutes to do a proper refactor, but here is just a sample of what could be done in half an hour; from 600+ lines to ~200: https://gist.github.com/mikeschinkel/8bb52534836e3cd19eee7d4f3f5fa9fd > You could split it up into multiple helper functions, but instead of hundreds > of lines in one function for a complicated file format, > you'd have to jump across even more abstractions/lines to know where XYZ was > set. I would argue that it is much harder to grok the entirety of the audio file by having to read a 600+ line function than if the main function called different functions that each handled a different part of the file. In refactoring the above, I found clear and distinct areas to separate for LAME encoding and different variable bit rate encoding types. Also, this function is begging to be refactoring to use a variety of object classes, even if the output is covered to arrays for backward compatibility. Instead of making it harder to understand the file format, breaking it out to multiple functions that mirror that different aspects of process a file will allow the developer to see the big picture — which is currently impossible with a 600 line function — and then to zoom into specific program functions when they want to understand more detail about a specific area. > Fully refactoring that might not make ever make business sense, > but there is a benefit to having the ability to mark if a variable is > actually block-scope or used elsewhere > (e.g. to have a better idea of if refactoring to a different function is safe) I totally get and agree with the concern about refactoring not making business sense. If you don't *need* to touch the code, don't touch the code. But if you are going to modify the file to add block level declarations then you can just as easily refactor the area you would modify into its own function. Leaving as one large function means that you will likely never fully understand the patterns at play, and thus will never be able to simplify maintenance. And will almost certainly have to play whack-a-mole with bugs each time someone modifies it. One thing is certain; you cannot reliably unit-test a long function like that; there is just too much going on to test in that function. This type of long function is what has plagued my current client and has caused them to spend about 80% of their developer time fixing bugs rather than devoting most of their time to build new features to help them grow their business (they brought me on specifically to fix that problem, so seeing you trying to empower developers to create more spaghetti like that made me feel the need to speak out. But again, it's just my opinion.) >> Given the nature of long functions, I think the awkward syntax here provides >> a nice disincentive to writing overly long functions. >> >> But as I said, the closure does exist in case someone absolutely needs to >> control >> the scope of a block within an existing long function, such as the 1400 line >> function I've been refactoring for months now... > > Also, closures aren't a convenient substitute for everything the local scope > can do for other reasons. > E.g. modifying by reference will mask issues with undefined or possibly > undefined variables when modifying values outside of the closure scope. > > ``` > function compute_product($values) { > // Other lines omitted > foreach ($values as $value) { > (function() use (&$product, $value) { > $letVar = $value + 1; > // Other lines omitted > $product *= $letVar; > })(); > } > return $product; > } > ``` > > This has the bug that $product was never defined, but no notices are emitted > because $product gets set to null > when the reference is taken by reference, and php currently doesn't warn > about multiplying by null. > > Using an actual `let $var = $value + 1`, this would get the notice for the > variable being undefined. This example is too contrived for me to determine if there is an easy alternative coded it in a clearer manner that exist that does not have the concerns you mention. At first blush I would have the closure return a value and then do the multiplication outside the closure which solves your stated issue, but then the closure really does nothing, or at least nothing without the omitted lines included. Still, I don't argue that you *should* use closures instead of refactoring to additional functions, just that they are there to use in case you are determined not to refactor into multiple functions. #fwiw -Mike -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
