init historian in ctor

But your way will break any 3rd party place manager that inherits from
PlaceManagerImpl because the constructor will change.  Is that ok?

On Fri, Oct 3, 2014 at 10:12 PM, Richard Wallis <rdwallis@gmail.com> wrote:

> lol, ok next time I'll wait for your lgtm.  I'll do another pr.
>
> On Fri, Oct 3, 2014 at 10:07 PM, Christopher Viel <
> notifications@github.com> wrote:
>
>> In
>> gwtp-core/gwtp-mvp-client/src/main/java/com/gwtplatform/mvp/client/proxy/PlaceManagerImpl.java:
>>
>> > @@ -57,8 +57,13 @@
>> >      private List<PlaceRequest> placeHierarchy = new ArrayList<PlaceRequest>();
>> >
>> >      public PlaceManagerImpl(EventBus eventBus, TokenFormatter tokenFormatter) {
>> > +        this(eventBus, tokenFormatter, (Historian) GWT.create(DefaultHistorian.class));
>>
>> This is closer to what I said, but it's lacking the core idea. Historian
>> was been introduced to make mocking easier. The 2nd ctor would help that,
>> though I don't see why PlaceManagerImpl (which is abstract) would
>> provide a default implementation. I would give that role to
>> DefaultPlaceManager. And since the latter has an injected ctor, you can get
>> rid of the GWT.create().
>>
>> DefaultPlaceManager extends PlaceManagerImpl {
>>  @Inject DefaultPlaceManager(EventBus eventBus, TokenFormatter tokenFormatter, DefaultHistorian historian) {
>>     super(eventBus, tokenFormatter, historian);
>>   }
>> }
>>
>> PlaceManagerImpl {
>>   PlaceManagerImpl(EventBus eventBus, TokenFormatter tokenFormatter, Historian historian) {/*snip*/}
>> }
>>
>> —
>> Reply to this email directly or view it on GitHub
>> <https://github.com/ArcBees/GWTP/pull/593/files#r18416753>.
>>
>
>