Hi,

Thanks for your feeback!

Tobias Schlitt wrote:
> 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.

Don't you mean that the request maker should run the filters?
Should the selection of filters depend on the controller or the request
builder?

> 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.

Sounds like a good idea, do you mean that a property like $request->data would
either be an array of variables, either a main content? What is your exact
proposal?

> 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.

Or add property $request->auth->location?

> ezcMvcRequestFile
> -----------------
> 
> This class defines an attribute $status. What is this meant for?

This is meant for $_FILES['userfile']['error']

> ezcMvcRequestUserAgent
> ----------------------
> 
> This struct only defines 1 attribute. Therefore it looks like overhead
> to me. We can simply store the user agent as a string in the
> ezcMvcRequest object. Or am I wrong?

Are we sure that it is the only attribute that we want?

> 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? 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
>     }
> }
> 
> ezcMvcHtmlViewSomething extends ezcMvcViewSomething
> {
>     public function handleResponse( ezcMvcHtmlResponse $response )
>     {
>         // Process a template and print the result...
>     }
> }

Actually, the template parsing would be done in createResponse(), which returns
an ezcMvcResponse struct, with the headers and body variables ready.

We could use ezcMvcHttpOutputWriter, which would set the headers and echo the
response body?

> ezcMvcController
> ----------------
> 
> So far the design defines that the action is selected internally by the
> controller. While I can live with this solution, I think the decision
> belongs more to the dispatcher. This avoids quite some duplication in
> controller code, I'd say.

Do you really prefer to set a mapping between controllers methods and request
variable 'action' or something like that?
Same for sub-actions?

Providing an abstract class that does that would work as well.

Regards, James.
-- 
Components mailing list
Components@lists.ez.no
http://lists.ez.no/mailman/listinfo/components

Reply via email to