Hi! On 06/30/2008 09:24 AM Derick Rethans wrote: > On Sun, 29 Jun 2008, Tobias Schlitt wrote:
>> ezcMvcRequestBuilder >> -------------------- >> >> I think the name of this class is a bit strange. I'd favor to call it >> ezcMvcRequestParser. I think this sounds more common. > That's the name we had previously, but as some people pointed out: This > class does not *parse* a request - it builds a request object from the > raw request data. RequestParser would sortof indicate it accepted a > request object already, instead of creating one. This is why we changed > it to ezcMvcRequestBuilder. I sort of disagree. In fact we should try to choose a name that a) reflects the class behavior properly and b) is commonly used. ezcMvcRequestParser sounds more familiar to me, not sure how far this is purely subjective. Beside that, an object of class actually receives raw request data and parses it. The result of this parsing is an ezcMvcRequest instance. For example, if you parse an incoming email as a request, you will most commonly use the Mail component to *parse* the raw email text and then use a custom *parser* to extract information from the mail body. With HTTP it is the same: PHP receives the request from the web server and *parses* the GET/POST/COOKIE/... arrays. The received request body will propably again *parsed* by a custom parser. >> ezcMvcRequestFilter >> ------------------- >> >> The filters are described to be used in a method named >> ezcMvcController->runRequestFilters(), while this method should be >> called by ezcMvcController->run(). It does make little sense to me to >> mention that a method ezcMvcController->runRequestFilters() should be >> implemented at all, if the controller calls the filter itself. >> >> Instead I would suggest to design it in either of the following ways: >> >> a) Remove the ezcMvcController->runRequestFilters() part and just >> mention that filters should be run by the controller. >> b) Require the method ezcMvcController->runRequestFilters() in the >> interface and make it be run by the request builder. > The reason for not calling the interface methods just run() is so that > you can implement all the three filter interfaces in one class: > ezcMvcAuthenticationFilter implements ezcRequestFilter, ezcResultFilter, > ezcResponseFilter. > The filters should indeed just be run by the controller from the run() > method in some way. I think you're right that we can just remove it. > (your point a). As for b, that would be a bad idea, as the request > builder doesn't actually run the controller, but just returns the > request object. The dispatcher calls the controller with this request > object. How the controller runs the filter, should not really matter - > however, I think it would be good to standardize it, and thus leave it > in the interface like we've currently designed. If we go for standardizing it, then why not simply make the dispatcher run that stuff? This avoids having all controllers run this methods. >> ezcMvcRequest >> ------------- >> >> The class defines a $content and a $variables attribute. Where is the >> difference here? >> >> In case of GET/POST requests, there are only variables, files, etc. So >> not content. In case of PUT requests, there might be content, but no >> variables. Shouldn't this be unified? In case of a SOAP request, there >> are only variables, too, but no content. > It's a struct, so I don't see what you're trying to say here. The point is, that $variables and $content is redundant. >> ezcMvcRequestAuthentication >> --------------------------- >> >> This class defines an $identifier and $password field. These fields only >> make sense for authentications as Basic-Auth. However, in case of e.g. >> OpenID, they are not available. Such differences should be abstracted in >> the class. > That's why this is just part of the stuct as a property of > ezcMvcRequest. It can be absent of course. The point is not the absense, but the different needs for authentication mechanisms. Is this struct meant to be extended for different authentication methods? As I said, OpenID is an example where you don't have username and password. Token based authentication is another example. Maybe someone wants to realize GPG key based authentication? >> ezcMvcRequestFile >> ----------------- >> >> This class defines an attribute $status. What is this meant for? > Upload status field, see: > http://php.net/manual/en/features.file-upload.errors.php I added this link to the design doc. >> ezcMvcViewHandler >> ----------------- >> >> The design states, that this class should implement createResponse() and >> handleResponse(). Both methods are just called in a chain. Is there much >> sense in defining both, then? > Yes, easier testability. >> It would only make sense, if createResponse() is defined in a >> different class than handleResponse(). Shouldn't this be different >> classes anyway? I imagine: >> >> ezcMvcProductHtmlViewHandler extends ezcMvcViewHandler >> { >> public function createResponse( ezcMvcResult $result ) >> { >> // Prepare the product result for HTML output... >> // ... return a HTML response object >> } >> } > The preparring is done by the controller, I don't see why you should > prepare the result from the controller in the view again... >> ezcMvcHtmlViewSomething extends ezcMvcViewSomething >> { >> public function handleResponse( ezcMvcHtmlResponse $response ) >> { >> // Process a template and print the result... >> } >> } > The handleResponse method should just send the output and set the > correct HTTP headers. The point is, that the controller just returns an abstract result object, that could potentially be send as any kind of response (HTML over HTTP, HTML stored to a file, SOAP response, Email). The view handler is then up to transform the result in such a way that it can be send as the protocol specific response (a template object - aka ezcMvcHtmlResponse for HTML output, an ezcMail object, a SOAP response,...). This view handler is somewhat dependant on the controller. Maybe one view handler can process the result of different controllers, but commonly you'd have multiple view handlers to process different result objects. The final serialization and outputing should be equal for all responses of a certain type (e.g. sending headers for HTTP response and printing the HTML code, opening a file handler and storing the HTML output, creating an MTA connection and sending the email,...). Therefore I'd devide these functionalities into different classes. One class to process the result to a certain output format, another one to handle the real sending of the response. >> Example Dispatcher >> ------------------ >> >> The example implementation uses goto. While I like that this is added to >> PHP in 5.3 (e.g. for parser implementations), I don't see a sense in >> using it here. We should better extract the re-run part into a method >> and call it recursively, if needed. > It was a joke ;-) *muhaha* However, I can imagine people would would write such code for PHP 5.3. >> The implementation also makes use of a $routerManager, which is not >> defined anyhwhere else. > You have to implement it, just like the viewManager. Both are at the top > of the example implementation of the dispatcher. Don't we define an interface here and describe what the classes implementing it are supposed to do? Would make sense, IMO. Regards, Toby -- Mit freundlichen Grüßen / Med vennlig hilsen / With kind regards Tobias Schlitt (GPG: 0xC462BC14) eZ Components Developer [EMAIL PROTECTED] | eZ Systems AS | ez.no -- Components mailing list Components@lists.ez.no http://lists.ez.no/mailman/listinfo/components
