Hi Larry, Thank you very much for your feedback! I think I have already partially answered some of your questions in my previous email to Niels, but let me answer your other questions below:
* I... do not understand the point of having public properties AND > getters/withers. A readonly class with withers, OK, a bit clunky to > implement but it would be your problem in C, not mine, so I don't care. > :-) But why getters AND public properties? If going that far, why not > finish up clone-with and then we don't need the withers, either? :-) I know it's disappointing, but the public modifiers are just a typo which were forgotten there from the very first iteration of the API :) However, I'm fine with having public readonly properties without getters as well, as long as we declare this a policy that we are going to adopt... Withers are indeed a must for now (and their implementation indeed requires some magic in C...). * Making all the parameters to Url required except port makes little sense > to me. User/pass is more likely to be omitted 99% of the time than port. > In practice, most components are optional, in which case it would be > inaccurate to not make them nullable. Empty string wouldn't be quite the > same, as that is still a value and code that knows to skip empty string > when doing something is basically the same as code that knows to skip > nulls. We should assume people are going to instantiate this class > themselves often, not just get it from the parser, so it should be designed > to support that. I may have misunderstood what you wrote, but all the parameters - including port - are required. If you really meant "nullable" instead of "required", then you are right. Apart from this, I'm completely fine with making these parameters optional, especially if we decide not to have the UrlParser (my initial assumption was that the Url class is going to be instantiated via UrlParser::parseUrl() calls). * I would not make Url final. "OMG but then people can extend it!" > Exactly. I can absolutely see a case for an HttpUrl subclass that enforces > scheme as http/https, or an FtpUrl that enforces a scheme of ftp, etc. Or > even an InternalUrl that assumes the host is one particular company, or > something. (If this sounds like scope creep, it's because I am confident > that people will want to creep this direction and we should plan ahead for > it.) Without having thought much about its consequences on the implementation, I'm fine with removing the final modifier. * If the intent of the withers is to mimic PSR-7, I don't think it does so > effectively. Without the interface, it couldn't be a drop-in replacement > for UriInterface anyway. And we cannot extend it to add the interface if > it's final. Widening the parameters in PSR-7 interfaces to support both > wouldn't work, as that would be a hard-BC break for any existing > implementations. So I don't really see what the goal is here. I've just answered this to Ben, but let me reiterate: PSR-7's UriInterface is only needed because PHP doesn't have a Url internal class. :) Máté
