Thanks for doing this, Rodrigo. I think people will find it very helpful.
Since it's sample code, I think it would be fine to add it to the 2.2 branch if you can get through this feedback in time. http://gwt-code-reviews.appspot.com/1299801/diff/1/2 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/PersonEditor.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/2#newcode59 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/PersonEditor.java:59: public PersonEditor(DynaTableRequestFactory factory) { If we're following the DI pattern, ScheduleEditor should be a constructor arg http://gwt-code-reviews.appspot.com/1299801/diff/1/4 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/ScheduleEditor.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/4#newcode41 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/ScheduleEditor.java:41: public ScheduleEditor(DynaTableRequestFactory factory) { timeSlots should be a constructor arg http://gwt-code-reviews.appspot.com/1299801/diff/1/5 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/ScheduleEditor.ui.xml (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/5#newcode6 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/ScheduleEditor.ui.xml:6: /* Add CSS here. See the GWT docs on UI Binder for more details */ This comment is noise, I think. http://gwt-code-reviews.appspot.com/1299801/diff/1/5#newcode7 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/ScheduleEditor.ui.xml:7: .important { Does anything use this style? http://gwt-code-reviews.appspot.com/1299801/diff/1/7 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/7#newcode64 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.java:64: class ScheduleRow { why not private? http://gwt-code-reviews.appspot.com/1299801/diff/1/7#newcode76 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.java:76: for (TimeSlotProxy timeSlot : backing) { This is kind of nasty. Could you make an immutable TimeSlotKey object that implemented this comparison in its equals, and keep a map of TimeSlotKey to TimeSlotProxy? http://gwt-code-reviews.appspot.com/1299801/diff/1/7#newcode96 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.java:96: context.createTimeSlot(day.ordinal(), hour * 60, hour * 60 + 50).fire(new Receiver<TimeSlotProxy>() { What happens on redundant request, if the user clicks a few times before the response is received? http://gwt-code-reviews.appspot.com/1299801/diff/1/7#newcode118 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.java:118: SATURDAY("S"); Two character short names would avoid ambiguity http://gwt-code-reviews.appspot.com/1299801/diff/1/8 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.ui.xml (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/8#newcode6 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.ui.xml:6: /* Add CSS here. See the GWT docs on UI Binder for more details */ ditto http://gwt-code-reviews.appspot.com/1299801/diff/1/8#newcode7 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/TimeSlotListWidget.ui.xml:7: .important { ditto http://gwt-code-reviews.appspot.com/1299801/diff/1/9 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/9#newcode154 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java:154: * to supply methods required by RequestFactory. This comment was screaming out "delete me!" If the persist method is no longer used, just kill it. If it is still used, please get rid of the "When this was written..." stuff, nobody cares about our history. And the "See...to supply methods required..." stuff won't make sense here since this method is not a requirement. http://gwt-code-reviews.appspot.com/1299801/diff/1/10 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Schedule.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/10#newcode25 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Schedule.java:25: * {@code getVersion()} nor {@code findSchedule()}. Those methods This entity does not follow the usual pattern of providing getId(), getVersion() and findSchedule() methods for RequestFactory's use. ScheduleLocator handles those responsibilities instead. http://gwt-code-reviews.appspot.com/1299801/diff/1/10#newcode26 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Schedule.java:26: * are provided by {@link ScheduleLocator} instead. This link won't work, it's not in the same package. http://gwt-code-reviews.appspot.com/1299801/diff/1/10#newcode85 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Schedule.java:85: this.timeSlots = timeSlots; should make a defensive copy http://gwt-code-reviews.appspot.com/1299801/diff/1/12 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/PersonFuzzer.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/12#newcode30 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/PersonFuzzer.java:30: static final int MAX_PEOPLE = 100; Might as well be public http://gwt-code-reviews.appspot.com/1299801/diff/1/12#newcode75 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/PersonFuzzer.java:75: if (rnd.nextInt(STUDENTS_PER_PROF) == 0) { Why? http://gwt-code-reviews.appspot.com/1299801/diff/1/14 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleFuzzer.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/14#newcode2 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleFuzzer.java:2: * Copyright 2007 Google Inc. 2011 http://gwt-code-reviews.appspot.com/1299801/diff/1/15 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleLocator.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/15#newcode2 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleLocator.java:2: * Copyright 2010 Google Inc. 2011 http://gwt-code-reviews.appspot.com/1299801/diff/1/15#newcode28 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleLocator.java:28: * A Schedule locator that interfaces between DTOs and Request Factory. This class serves as an example of implementing a Locator to allow RequestFactory to work with entities that don't conform to its expectations of static find*() methods, and getId() and getVersion() methods. In a production application such a Locator might be the bridge to your dependency injection framework, or a data access object. <p> There is a reference to this class in a {@literal @}Service annotation in {@link ...DynaTableRequestFactory} http://gwt-code-reviews.appspot.com/1299801/diff/1/15#newcode30 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleLocator.java:30: public class ScheduleLocator extends Locator<Schedule, String> { I think the example would be clearer if you refactored a ScheduleSource out of the ScheduleLocator, something analogous to PersonSource. ScheduleCalendarService would know about ScheduleSource, not ScheduleLocator. The locator would be a very simple thing, one step up from a configuration file. http://gwt-code-reviews.appspot.com/1299801/diff/1/16 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleService.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/16#newcode21 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleService.java:21: * Service object for Schedule entities. , used to demonstrate the use of non-static service objects with RequestFactory. RequestFactory finds this service via the {@link ScheduleServiceLocator}. http://gwt-code-reviews.appspot.com/1299801/diff/1/17 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleServiceLocator.java (right): http://gwt-code-reviews.appspot.com/1299801/diff/1/17#newcode21 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/ScheduleServiceLocator.java:21: * Locator for {@link Schedule} service objects. This class provides an example of implementing a ServiceLocator to allow RequestFactory to work with instances of service objects, instead of its default behavior of mapping service calls to static methods. There is a reference to this class in an {@literal @}Service annotation in {@link ...DynaTableRequestFactory} http://gwt-code-reviews.appspot.com/1299801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
