On Sat, Mar 25, 2017 at 4:40 AM, Levi Morrison <le...@php.net> wrote:
> On Fri, Mar 24, 2017 at 1:04 PM, Nikita Popov <nikita....@gmail.com> > wrote: > > On Fri, Mar 24, 2017 at 5:26 PM, Levi Morrison <le...@php.net> wrote: > >> I prefer a distinct function/method from `token_get_all`. I don't see > >> the value in having a return value that differs so much based on a > >> passed flag. I don't care so much about a function vs static method; I > >> just oppose the additional parameter to `token_get_all`. > >> > >> I like the idea very much overall aside from that detail. > > > > > > token_get_all() already has a $flags parameter controlling its output. > > Currently only the TOKEN_PARSE flag is supported, which changes the > > interpretation of certain tokens. In a previous thread on this topic Sara > > suggested another flag (something like TOKEN_ALWAYS_ARRAY) which makes > the > > output of token_get_all() consistently use arrays. I think handling this > as > > a flag is entirely reasonable, and by analogy, handling the object mode > as a > > flag is also reasonable. In the end, these are just minor variations on > the > > output format (minor relative to the very complex primary functionality > of > > the function: lexing). Another precedent is the json_decode() option to > use > > arrays or objects in the return value. > > > > Regards, > > Nikita > > Whether TOKEN_PARSE is passed or not the return type is Array<char | > Array>. You are proposing an entirely different return structure of > Array<PhpToken>. What are the benefits of reusing the same function > have? To me adjusting a parameter and getting a totally different > structure is not a benefit, so there must be some other unspecified > advantage you haven't articulated yet. If there is a benefit to it > great; please say so. Otherwise I don't see why you want to make an > existing function more complicated when you can just make a different > function. > (cc'ing back internals) The benefit that I was alluding to in my previous reply, though failed to state explicitly, is essentially forward compatibility, in the sense of robustness to extension. We already have an existing flag controlling behavior, I am proposing to add another (whether or not it be implemented as an actual flag -- it is a flag in the sense of a combinatorial boolean option), and it is not unreasonable to expect that more such options may appear in the future. It is inconsistent if some of these options are implemented as flags, while others are separate functions. Implementing everything as separate functions results in combinatorial explosion. Having the variants token_get_all(), token_get_all_as_object(), token_get_all_with_positions(), token_get_all_as_object_with_positions() is certainly not an enviable circumstance to be in. Of course, you may argue that "as object" and "with positions" are different because one changes the return type while the other doesn't (in the particularly limited typesystem you happen to be using -- a more accurate description would be array<int, char|tuple<int, string, int>> vs array<int, char|tuple<int, string, int, int>>), so let's pick another example that can not be implemented as flag: The token_get_all_from_file() variant that Fleshgrinder has suggested (under a different name). While I do not personally believe this to be useful, there are some technical arguments that can be made in favor of having such a function (our lexer is capable of lexing directly from a file more efficiently). Now again, if we implement the AS_OBJECT mode using a separate function, this would require having all of token_get_all(), token_get_all_as_object(), token_get_all_from_file() and token_get_all_from_file_as_object(). Whether or not any of these specific examples are going to happen, I hope this serves to illustrate the general hazard of implementing flags (in the sense of combinatorial boolean options) as separate functions. As a more meta-level commentary: While I can appreciate the philosophy of preferring new functions over extended arguments, this is certainly not the first time this discussion has been had on this list. The end result tends to be the same: Whatever your personal philosophy might be, the existing corpus of PHP functions has a very strong preference towards extending existing functions rather than introducing new ones. Overall, I must say that this thread has left me rather disappointed. I was harboring the misguided hope that it might be possible to introduce a simple, self-contained improvement, that resolves a major ergonomics pain point in ext/tokenizer, while also improving performance and memory usage. Essentially, a win on all fronts. What I get instead is a continued escalation of this simple change, first into the addition of a new function (which I disagree with, but which is still a feasible suggestion), then into the addition of a complex object-oriented system involving three classes, at least six methods, iterators and even managing to involve the question of namespaced core functionality, a well known and extensively discussed unresolved question! What am I supposed to do with such suggestions? I will obviously not waste my time discussing the design of such a system, let alone implementing it in its glorious complexity. This is the point where I abandon this particular idea, as clearly my expectations of the scope of this change do not line up with those of others. I really wish people could get their priorities straight. Nikita
