Hi Jordi, In the examples you mention the usage of `parse` and `toString`. I assume > there is no “default” and this is a placeholder for one of the given > implementations. But it’s perhaps better to pick one of for the examples as > it suggests it would also exist. >
Ah, thanks for the info! Most of these were omission in fact, as I forgot to update the original method names after I unified the two QueryParams classes. > When it comes to the implementation I believe it would be nice to also > have a `getKeys()` method. That would only return the keys. > > You do mention the focus is to move away from $_GET. I like the idea, but > implementing QueryString would still require something like the following: > > Uri\QueryParams::fromArray($_GET); > > If we really want to make `$_GET` obsolete, wouldn’t it be nice to have a > `fromRequest()` which would directly parse `$_GET` or use > `$_SERVER["QUERY_STRING”]`. > As far as I can see, Uri\QueryParams::fromArray($_GET); wouldn't be needed. Do you have any use-case in mind where the suggested "Uri\QueryParams::parseRfc3986($_SERVER["QUERY_STRING"]);" call wouldn't work instead? I'm not against adding a fromRequest() method though if others find it useful (I'm a bit neutral about it). And I'd probably use a fromCurrentRequest() or a similar name to highlight the fact that it uses the current request as source. > As for whether or not this should be a readonly class. I think it should > be. The class itself is following various specs. And since all other > classes in the same namespace are also readonly, it should follow the same > principle (also for consistency) I believe. > I think the most problematic case is to return a mutable class from a readonly class (e.g. Uri\Rfc3986\Uri::getQueryParams()). So we should either omit this method, or make Uri\QueryParams readonly indeed. Regards, Máté
