On 23 août, 14:49, Patrick Julien <[email protected]> wrote:
> On Mon, Aug 23, 2010 at 5:16 AM, Thomas Broyer <[email protected]> wrote:
>
> > On 21 août, 20:13, Patrick Julien <[email protected]> wrote:
> >> I've been experimenting with PlaceHistoryManager and company.  So far,
> >> I have the following comments:
>
> > First, I've only read through the design-doc Wave and the code, I
> > haven't yet experimented with it (as I first have to migrate my
> > project from M2 to M3)
>
> >> 1. The tokens are big, they include the full path to the class, so all
> >> packages and the name of the class.  It's a lot.  In our case, it also
> >> exposes the name of the contractor to the user of the application.
>
> > This is only the "default" Proxy{,List}Place.Tokenizer
> > implementations; I put "default" between quotes as it's not even a
> > default, you have to *explicitly* use the tokenizers (the scaffold app
> > does this implicitly by using a PlaceHistoryHandlerWithFactory which
> > analyzes the so-called factory class looking for methods taking no
> > argument and returning PlaceTokenizer instances).
> > It looks like you're absolutely free to use any PlaceTokenizer
> > implementation you want, even with the stock Proxy{,List}Place places.
>
> No it's not.  This is the tokens returns by RequestFactory based on
> your record classes.  The *Place objects that you use have no bearing
> on what these methods produce.  The place tokenizer has nothing to do
> with this other that it uses RequestFactory to get these tokens and
> get back a Class<?> back from the tokens.

I'm sorry but I don't understand.
1. Ray C said that the goal was that there is some obfuscation going
on, to be added later on what RequestFactory.getToken/getProxy returns/
expects.
2. I added that this is only used by ProxyPlace.Tokenizer (and I
really see RequestFactory.getToken/getProxy as helper methods for
ProxyPlace.Tokenizer), that you're free to *not* use.
I understand this is feedback on RequestFactory.getToken return values
(though it wasn't very clear) but yet again, Ray C responded to your
complaint and in case you'd like to deliver a version of your app
without waiting for this to be fixed/enhanced, you can workaround it
by providing your own PlaceTokenizer.

> >> 2. Tokens in RequestFactory only work with Record based classes.  I
> >> think that's a shame.  Will another facility supplement this?  Because
> >> if tokens from gwt will only support Record, why even bother with
> >> ProxyPlace?  Just make a goTo method in PlaceController that takes a
> >> record.  I would prefer to have tokens for a place directly.  I think
> >> that can be accomplished by having multiple place tokenizers however,
> >> one per place.
>
> > I don't understand. Of course RequestFactory only deals with Records,
> > but that's totally independent from Places and History tokens (well,
>
> Why "of course"?  RequestFactory means a factory of "requests", not a
> factory of "records".  You can use RequestFactory to fire requests
> that don't have any records, e.g.,
>
> RequestObject<Long> getMeALongNotUsingARecord(Long param1, Long param2);
>
> no [record] here.

Yes, you can, but (correct me if I'm wrong):
1. anything more complex than a primitive type or its wrapper class, a
Date, or a String has to be a Record
2. you'd have to make your own Place and PlaceTokenizer for those, so
why bother asking RequestFactory for a token? (have I said I do think
getToken/getProxy are really "just" helpers for
ProxyPlace.Tokenizer? ;-) )
You used the expression "tokens from GWT", which I understood as
"history management", which *of course* are not limited to records; so
there's really a reason to "bother" with ProxyPlace.
Having a goTo(Record) in PlaceController would make it depend on
RequestFactory, which would be bad design (as long as you don't use
ProxyPlace and ProxyListPlace, you can very well use PlaceController
and PlaceHistoryHandler without using RequestFactory at all).
I'd add that having a PlaceTokenizer *per place* is by-design; this is
very different from gwt-presenter which has a single PlaceFormatter to
handle all places.

> > I haven't seen any limitation in PlaceHistoryHandler forbidding its
> > use with places other the the provided Proxy{,List}Place; of course,
> > for those places, you'd have to provide your own PlaceTokenizer.
>
> That's what I am currently doing.

have you tried using ProxyPlace (provided what it represents –an
"operation" on a particular record– suits your needs) with a custom
PlaceTokenizer?

> >> 3. Ultimately, I don't think the assumption that a record type is 1:1
> >> to an activity.  The mapper stuff allows for this but is yet more
> >> code.
>
> > There's no such assumption AFAICT. And actually, for a given record
> > type, ProxyPlace gives you 3 different places, and ProxyListPlace an
> > additional one. You're then free to map those places to activities in
> > your ActivityMapper. And of course you're free to not use Proxy{,List}
> > Place at all.
>
> There's is such an assumption in ProxyPlace because it requires a
> record

ProxyPlace (as I said above) represents "an operation on a record", so
it of course requires a record. Not all places are record-related, but
in this case you'd use another Place than a ProxyPlace.
Let's say you have a "dashboard" as your "home page", you'll then
create a DashboardPlace and its associated PlaceTokenizer. If you also
have an "about page", you'll create an AboutPlace and its associated
PlaceTokenizer, etc. (note that technically, you could also use a
single PlaceTokenizer for multiple places if you liked)
A Place answers the question "where am I?", a ProxyPlace is an answer
of the form "doing something on this record", where "doing something"
can be "creating", "seeing the details" or "editing", and "this
record" being a record instance; and a ProxyListPlace is an answer of
the form "a list of records of type X". Again, this is different from
gwt-presenter (where you use a singleton Place per presenter –speaking
from the examples, I cannot understand the exact flow of events/
actions in gwt-presenter "trunk"–, and where a Place instance actually
represents "some screen/activity", and the place has some added
"state" the describes the actual "data" being displayed/edited/
whatever).

> The other reason why that assumption is made is that
> AbstractRecordEditActivity requires a record now.  Currently, there is
> no way to build a proxy record, i.e., a record where all fields are
> stale but the id, in code.

Yes you can: use requestFactory.create()
http://code.google.com/p/google-web-toolkit/source/browse/trunk/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java#125

> >> 5. I think the Place class getting support for a collection of arguments, 
> >> i.e.,
>
> >> setArgument(String, String);
> >> String getArgument(String);
> >> clearArguments();
>
> >> and more importantly, parseArguments(String s); so that we can pass in
> >> a string from the PlaceTokenizer.getPlace() after we've removed the
> >> prefix would be most helpful and be applicable to most.
>
> > Please NOOOO!!! That's the role of the PlaceTokenizer. If you want a
> > generic place with String "arguments" and a generic tokenizer making
> > look like e.g. a query-string in history tokens, you can do it already
> > but please don't make it a default!
> > You shouldn't do getArgument("foo"), you should instead have a
> > specific Place subclass giving you a getFoo().
>
> Because you're assuming that one place and one list place is
> sufficient for all needs and I don't think it is.

Neither do I!

> You have common
> code dealing with arguments that will always be there.  We can have a
> base place that adds it and does it on our own but would still be
> practical to have in Place since it's currently empty.
>
> The argument for these common methods for arguments is that at the
> very least, they have to be placed on the url bar and be parsed back.
> That is common to all places.
>
> Furthermore, yes, getFoo() instead of getArgument("foo") is nice but
>
> a) It doesn't mean a base place with getArgument("foo") can't have a
> concrete class that has getFoo() that does the proper construction
> b) With ActivityMapper no longer being a generic, getting to getFoo()
> means downcasting all over the code.

Yes, ActivityMappers are expected to be made of if/else around
instanceof tests. And actually, Places are only expected to be used
from ActivityMappers (the ActivityMapper will setup the Activity with
the appropriate "state", so an Activity doesn't need to know the
Place, and I'd even say it shouldn't know about places at all, except
for instantiating them to give them to PlaceController.goTo).
See for example
http://code.google.com/p/google-web-toolkit/source/browse/trunk/bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesDetailsActivities.java
(which only deals with ProxyPlace, but still does a check at the very
start to make sure it doesn't deal with anything else; and the
ExpensesEntityTypesProcessor (this is an implementation detail of apps
generated by Roo; I guess it makes them easier to maintain by Roo when
you add record types) does the same with the record types:
http://code.google.com/p/google-web-toolkit/source/browse/trunk/bikeshed/src/com/google/gwt/sample/expenses/gwt/request/ExpensesEntityTypesProcessor.java#67

> To address your comment that this is the role of PlaceTokenizer, it
> is, but currently, there is no standard PlaceTokenizer that handles
> standard query parameters.

And I don't think there should be. Though I think it should be easy to
build one, and "frameworks" like gwt-presenter would probably given
one to you.


> And there can't be one unless it has an
> interface in which it get and set place arguments into the place
> itself, hence the argument for get/setArgument in Place.

public class GenericPlace extends Place {
   private final String name;
   private final Map<String,String> parameters;

   // ctor and getters here...

  public static class Tokenizer extends PlaceTokenizer<GenericPlace> {
    public GenericPlace getPlace(String token) {
      // parse the token and return a new GenericPlace(...)
    }
    public String getToken(GenericPlace place) {
      // build the <name>?<key1>=<value1>&<key2>=<value2>&... from the
GenericPlace
    }
  }
}

Now, you just have to use GenericPlace all over the place (and yes, it
means starting your ActivityMappers with an "if (!(place isntanceof
GenericPlace)) return;" statement) and code everything in a non-type-
safe manner with string comparisons everywhere.

The intended use is that you discriminate the kinds of places based on
a token prefix, so GWT takes care of the dispatch and only calls the
"appropriate" PlaceTokenizer for a given kind of Place. You can of
course do your own dispatching in a single "root" (unprefixed)
PlaceTokenizer, but that means (a little bit) more code.
And on serializing a Place to a token, GWT will match the
PlaceTokenizer based on its generic type argument (code generation
FTW!)

> > It may sound a bit harsh but you've IMO been very badly influenced by
> > gwt-presenter.
>
> Is that really necessary?  We're building out a real, big application
> and giving back feedback.

I'm sorry, this was more directed to gwt-presenter than to you
personnaly. As I said, I really dislike how gwt-presenter deals with
places, and I think it doesn't "get" the concept of places.
The reason I'm supporting GWT's approach is that the one I developed a
year ago for our own apps is very similar (minus the notion of
activities, but at the lower-level of Place, PlaceController,
PlaceChangeEvent, and now PlaceTokenizer, there are many things in
common).

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to