Sample project demonstrating the usage of DispatchAsync without GWT-RPC

[jgonian, 2010-09-13 18:34:30]

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/impl/CreateItemActionHandler.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/impl/CreateItemActionHandler.java:35:
}
I think it would be more clear if the status code of the response is checked
instead of the payload, e.g.

       switch (response.getStatusCode()) {
            case Response.SC_OK:
                success = true;
                break;
            default:
                success = false;
                break;
        }

or, 
final boolean success = (response.getStatusCode() == Response.SC_OK);
return new CreateItemActionResult(success);

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/impl/CreateItemActionHandler.java:48:
return new RequestBuilder(RequestBuilder.GET, url);
The example would be more instructive if the create action was performed with a
POST (or maybe PUT) request.

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/impl/GetAllItemsActionHandler.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/impl/GetAllItemsActionHandler.java:34:

No error handling and no check for successful response (i.e. 200 OK) before
processing the payload. 
In case the response does not contain expected json (e.g. a server error
occurred) a Javascript exception will be thrown.

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/impl/GetAllItemsActionHandler.java:55:
};
Unnecessary semicolon

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/impl/GetAllItemsActionHandler.java:57:
private final native JsArray<Item> asArrayOfItem(String json) /*-{
Declaring a private method as final is redundant

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/dispatch/RequestBuilderActionHandler.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/dispatch/RequestBuilderActionHandler.java:31:
*/
Incomplete javadoc.

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/dispatch/RequestBuilderActionHandler.java:33:

Missing javadoc.

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/dispatch/RequestBuilderActionHandler.java:38:
*/
Incomplete javadoc.

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/dispatch/RequestRuntimeException.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/dispatch/RequestRuntimeException.java:24:
@SuppressWarnings("serial")
Missing serialVersionUID field e.g.
private static final long serialVersionUID = -12345L;

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/gin/MyModule.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/gin/MyModule.java:42:
bind(TokenFormatter.class).to(ParameterTokenFormatter.class).in(
Inconsistent folding. See line 46.

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/model/Item.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/model/Item.java:22:
public class Item extends JavaScriptObject {
I believe it is a good practice for models to implement an interface. Otherwise
it is not possible to use them in plain junit tests.
Consider extracting an interface for this model and renaming this class to
ItemJsoImpl.

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/server/ItemsManagerServlet.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/server/ItemsManagerServlet.java:31:
@SuppressWarnings("serial")
A serialVersionUID field would do the job.

[marius.andreiana, 2010-09-13 19:25:17]

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/MainPagePresenter.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/MainPagePresenter.java:63:
class CreateClickHandler implements ClickHandler {
In order to write less code, instead of this class, could the view call directly
the dispatch() or call a presenter UiHandler method which does this dispatch? (I
think the latter would be best, so we don't have logic in the view)

See 
http://code.google.com/p/gwt-platform/source/browse/hplacesample/src/com/gwtp...

@UiHandler("all")
  void onAllClicked(ClickEvent event) {
    getUiHandlers().revealAllProductsList();
  }

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
File
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/CreateItemActionResult.java
(right):

http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/CreateItemActionResult.java:20:
public class CreateItemActionResult implements Result {
In order to keep the existing convention at
http://code.google.com/p/gwt-platform/wiki/BoilerplateGeneration#Generate_Act...

could this class be named CreateItemResult ?

This also matches the classes at
http://code.google.com/p/gwt-platform/source/browse/src/com/gwtplatform/dispa...
(Action and Result, for each request)

[denis.labaye, 2010-09-14 21:28:00]

A quick comment not directly related to code review : 
This sample has been updated, and a CR requested here :
http://codereview.appspot.com/2202041/

The update is : 
* use gwtp-0.4 instead of gwtp-0.3
* use JSON to communicate with the server
* use piriti as a parsing lib (for JSON/

On 2010/09/13 19:25:17, marius.andreiana wrote:
>
http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
> File
>
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/MainPagePresenter.java
> (right):
> 
>
http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
>
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/MainPagePresenter.java:63:
> class CreateClickHandler implements ClickHandler {
> In order to write less code, instead of this class, could the view call
directly
> the dispatch() or call a presenter UiHandler method which does this dispatch?
(I
> think the latter would be best, so we don't have logic in the view)
> 
> See 
>
http://code.google.com/p/gwt-platform/source/browse/hplacesample/src/com/gwtp...
> 
> @UiHandler("all")
>   void onAllClicked(ClickEvent event) {
>     getUiHandlers().revealAllProductsList();
>   }
> 
>
http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
> File
>
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/CreateItemActionResult.java
> (right):
> 
>
http://codereview.appspot.com/2171043/diff/1/dispatch-without-gwtrpc/src/com/...
>
dispatch-without-gwtrpc/src/com/gwtplatform/samples/dispatchwithoutgwtrpc/client/command/CreateItemActionResult.java:20:
> public class CreateItemActionResult implements Result {
> In order to keep the existing convention at
>
http://code.google.com/p/gwt-platform/wiki/BoilerplateGeneration#Generate_Act...
> 
> could this class be named CreateItemResult ?
> 
> This also matches the classes at
>
http://code.google.com/p/gwt-platform/source/browse/src/com/gwtplatform/dispa...
> (Action and Result, for each request)