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.
>
>> 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 request here.
> RequestFactory can give you tokens for your records, but yet again,
> that's only if you *want* to use that, which you'll do by using the
> provided Proxy{,List}Place.Tokenizer).
Again, hence my objection.
> 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.
>
>> 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 but more to the point because of AbstractRecordEditActivity.
If you have a user record and you want to edit your record, your user
record edit activity should be a function of your user privileges.
Currently, the only way to do this is to use activity mappers or use
multiple places. Again, I stated this.
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. You would need to create a string token
manually which assumes that:
a) You know what the string token looks like, append the id, and pass
it on to RequestFactory.
b) The string token algorithm will not change from version to version.
Already on this thread, we can see that it will already change.
c) or, fire the list request before you create the
AbstractRecordEditActivity, get a fresh record from the server and
have AbstractRecordEditActivity fire a list yet again once it's
constructed.
Due to AbstractRecordEditActivity, we're pretty much forced to carry
around records in the place.
>
>> 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. 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.
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 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.
> 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.
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
