You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by Phil Harvey <ph...@philharveyonline.com> on 2013/01/23 17:36:06 UTC
Reducing the visibility of proton-j constructors
As part of the Proton JNI work, I would like to remove all calls to
proton-j implementation constructors from "client code". I intend that
factories will be used instead [1], thereby abstracting away whether the
implementation is pure Java or proton-c-via-JNI.
I'd like to check that folks are happy with me making this change, and to
mention a couple of problems I've had.
In this context, "client code" is anything outside the current
sub-component, where our sub-components are Engine, Codec, Driver, Message
and Messenger, plus each of the contrib modules, and of course third party
code.
To enforce this abstraction, I am planning to make the constructors of the
affected classes package-private where possible. I believe that, although
third party projects might already be calling these constructors, it is
acceptable for us to change its public API in this manner while Proton is
such a young project.
Please shout if you disagree with any of the above.
Now, onto my problem. I started off with the org.apache.qpid.proton.engine.
impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport calls
various methods on ConnectionImpl and TransportImpl, so simply using a
Connection and Transport will not work. I don't know what to do about
this, and would welcome people's opinions.
Thanks
Phil
[1] for example, these work-in-progress classes:
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/ProtonFactoryLoader.java
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/engine/EngineFactory.java
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton/src/main/java/org/apache/qpid/proton/engine/impl/EngineFactoryImpl.java
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-c/bindings/java/jni/src/main/java/org/apache/qpid/proton/engine/jni/JNIEngineFactory.java
Re: Reducing the visibility of proton-j constructors
Posted by Rajith Attapattu <ra...@gmail.com>.
I agree with Rob on this point. Given the trouble we had with our old
client it will be fool hardy not to correct those mistake in the new
implementation.
We've also had instances where users (and sometimes our own test
cases) directly creating implementation classes using the public
constructors making it difficult to make any changes.
I also think Phil has a great point about API versioning.
Allowing access via Factories will afford us a lot of leeway going forward.
Rajith
On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <ro...@gmail.com> wrote:
> In general dependency on implementation is bad, dependency on interface is
> better. One of the problems we've had with Qpid historically is it become
> hard to test as we have dependencies on implementation everywhere.
>
> From an end user perspective the difference is only replacing
>
> X x = new X();
>
> with
>
> X x = XFactory.newInstance();
>
> But it makes it a lot easier to properly describe the functionality being
> returned. For implementation reasons, the concrete class may have public
> methods that are not intended to be exposed to the application programmer
> but only to other cooperating classes. To avoid confusion, and to make it
> clear to us the delta between the "shared" API and the API extensions that
> we are supporting for the pure Java implementation the interfaces seem like
> the right way to go for me.
>
> The idea is that those who need the extension features will still have a
> very clear way to get the implementation that provides these, without ever
> having to cast.
>
> -- Rob
>
>
>
>
>
> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
>
>> On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <rob.j.godfrey@gmail.com
>> >wrote:
>>
>> > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com> wrote:
>> > >
>> > > As part of the Proton JNI work, I would like to remove all calls to
>> > > proton-j implementation constructors from "client code". I intend that
>> > > factories will be used instead [1], thereby abstracting away whether
>> the
>> > > implementation is pure Java or proton-c-via-JNI.
>> > >
>> > > I'd like to check that folks are happy with me making this change, and
>> to
>> > > mention a couple of problems I've had.
>> > >
>> > > In this context, "client code" is anything outside the current
>> > > sub-component, where our sub-components are Engine, Codec, Driver,
>> > Message
>> > > and Messenger, plus each of the contrib modules, and of course third
>> > party
>> > > code.
>> > >
>> > > To enforce this abstraction, I am planning to make the constructors of
>> > the
>> > > affected classes package-private where possible. I believe that,
>> > although
>> > > third party projects might already be calling these constructors, it is
>> > > acceptable for us to change its public API in this manner while Proton
>> is
>> > > such a young project.
>> > >
>> >
>> > +1 to all of the above
>> >
>> > >
>> > > Please shout if you disagree with any of the above.
>> > >
>> > > Now, onto my problem. I started off with the
>> > org.apache.qpid.proton.engine.
>> > > impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
>> > calls
>> > > various methods on ConnectionImpl and TransportImpl, so simply using a
>> > > Connection and Transport will not work. I don't know what to do about
>> > > this, and would welcome people's opinions.
>> > >
>> >
>> > So, the simplest change would be to change the factories to use
>> > covariant return types
>> >
>> > e.g. EngingFactoryImpl becomes:
>> >
>> > @Override
>> > public ConnectionImpl createConnection()
>> > {
>> > return new ConnectionImpl();
>> > }
>> >
>> > @Override
>> > public TransportImpl createTransport()
>> > {
>> > return new TransportImpl();
>> > }
>> >
>> >
>> > ... etc
>> >
>> > Code that requires the extended functionality offered by the pure Java
>> > implementation can thus instantiate the desired Factory directly.
>> >
>>
>> What's the point of going through the factory in this scenario rather than
>> directly instantiating the classes as Hiram suggests? Is there some class
>> of thing the factory would/could do that the constructor can't/shouldn't?
>>
>> A second refinement might be to actually separate out the interface
>> > and implementation within the pure Java implementation so that we have
>> > a well defined "extended" Java API. This interface could then be the
>> > return type of the factory.
>> >
>>
>> Maybe I'm misunderstanding, but what's the point of using an interface here
>> if you're still locked into the pure Java impl? Are you expecting to swap
>> out that impl under some circumstances?
>>
>> --Rafael
>>
Re: Reducing the visibility of proton-j constructors
Posted by Hiram Chirino <hi...@hiramchirino.com>.
I'm using proton in more that one place :)
So the proton-hawtdispatch module gets used by Apollo and the
amqp-benchmark projects, but ActiveMQ 5.x use proton more directly.
On Mon, Jan 28, 2013 at 8:21 AM, Phil Harvey <ph...@philharveyonline.com>wrote:
> Yeah, I don't have a problem with doing this for Proton 0.4 if people think
> the API change caused by our constructor visibility reduction is too
> abrupt.
>
> I don't think this is strictly necessary for Hiram's code to continue
> working, since Keith and I are modifying it to use factories. This is
> already done on the JNI branch - take a look at
>
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/contrib
> .
> However, I understand there may be other projects already calling these
> constructors so accept we need to be polite to our users.
>
> We'd put @SuppressWarnings("deprecation") in EngineFactoryImpl so that its
> (valid) use of the constructors wouldn't cause compiler warnings.
>
> We would then make the constructors package-private in the followong
> release - Proton 0.5.
>
> Any objections?
>
> Phil
>
>
> On 28 January 2013 11:40, Rafael Schloming <rh...@alum.mit.edu> wrote:
>
> > Would it be better if we were to deprecate the constructors and remove
> them
> > in a future release?
> >
> > --Rafael
> >
> > On Fri, Jan 25, 2013 at 6:25 PM, Hiram Chirino <hiram@hiramchirino.com
> > >wrote:
> >
> > > I was hoping I could upgrade to newer proton versions without having to
> > > change my client code, since I was not planning on using the JNI impl
> > > anyways.
> > >
> > >
> > > On Fri, Jan 25, 2013 at 4:24 PM, Rajith Attapattu <rajith77@gmail.com
> > > >wrote:
> > >
> > > > This is also my impression about Hirams work.
> > > > So I'm not sure why there is resistance to the changes being proposed
> > > > as it's only going to benefit in the long run should the impl
> changes.
> > > > (I do understand that there will be some initial work in changing the
> > > > code to use the factories and interfaces).
> > > >
> > > > I also thought part of Phil's (and Rob's) work was to come up with a
> > > > set of interfaces for the API.
> > > > If I'm not mistaken this work is already sitting in a branch ? (I
> > > > stand to be corrected though).
> > > >
> > > > Rajith
> > > >
> > > > On Fri, Jan 25, 2013 at 3:44 PM, Rafael Schloming <rh...@alum.mit.edu>
> > > > wrote:
> > > > > FWIW, I don't think Hiram's usage fundamentally needs to depend on
> > the
> > > > pure
> > > > > Java impl. I may be out of date on this one, but I believe his
> access
> > > of
> > > > > the implementation was done for expediency. I know at least in a
> > number
> > > > of
> > > > > cases we discussed how the C implementation could accommodate his
> > > > > requirements, and we plan do to so at some point. Perhaps our
> efforts
> > > > here
> > > > > would be better spent in building out the interface so that he
> > doesn't
> > > > need
> > > > > direct access to the implementation.
> > > > >
> > > > > --Rafael
> > > > >
> > > > > On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <
> > > phil@philharveyonline.com
> > > > >wrote:
> > > > >
> > > > >> This has implications for API versioning,
> > > > >>
> > > > >> I think developers of third party code can assume that the API of
> > > public
> > > > >> methods, including constructors, will remain stable across minor
> > > > revisions
> > > > >> of a library. If a constructor is public then we, the Proton
> > > > developers,
> > > > >> have an obligation to respect this assumption (or at least to add
> > "for
> > > > >> internal use only" to its Javadoc).
> > > > >>
> > > > >> On reflection, I'm coming round to the idea that keeping these
> > > > constructors
> > > > >> public will place unreasonable restrictions on our ability to
> evolve
> > > the
> > > > >> implementation whilst keeping the API stable.
> > > > >>
> > > > >> Phil
> > > > >>
> > > > >>
> > > > >> On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com>
> > > wrote:
> > > > >>
> > > > >> > What's your need for the direct constructor?
> > > > >> >
> > > > >> >
> > > > >> > -- Rob
> > > > >> >
> > > > >> > On 25 January 2013 15:49, Hiram Chirino <hiram@hiramchirino.com
> >
> > > > wrote:
> > > > >> > > Let me clarify, I have no problem with adding Factories and
> > using
> > > > them
> > > > >> > > everywhere possible. Just don't take my access away from
> direct
> > > > >> > > constructors.
> > > > >> > >
> > > > >> > >
> > > > >> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <
> > > > rob.j.godfrey@gmail.com
> > > > >> > >wrote:
> > > > >> > >
> > > > >> > >> In general dependency on implementation is bad, dependency on
> > > > >> interface
> > > > >> > is
> > > > >> > >> better. One of the problems we've had with Qpid historically
> > is
> > > it
> > > > >> > become
> > > > >> > >> hard to test as we have dependencies on implementation
> > > everywhere.
> > > > >> > >>
> > > > >> > >> From an end user perspective the difference is only replacing
> > > > >> > >>
> > > > >> > >> X x = new X();
> > > > >> > >>
> > > > >> > >> with
> > > > >> > >>
> > > > >> > >> X x = XFactory.newInstance();
> > > > >> > >>
> > > > >> > >> But it makes it a lot easier to properly describe the
> > > functionality
> > > > >> > being
> > > > >> > >> returned. For implementation reasons, the concrete class may
> > have
> > > > >> public
> > > > >> > >> methods that are not intended to be exposed to the
> application
> > > > >> > programmer
> > > > >> > >> but only to other cooperating classes. To avoid confusion,
> and
> > > to
> > > > >> make
> > > > >> > it
> > > > >> > >> clear to us the delta between the "shared" API and the API
> > > > extensions
> > > > >> > that
> > > > >> > >> we are supporting for the pure Java implementation the
> > interfaces
> > > > seem
> > > > >> > like
> > > > >> > >> the right way to go for me.
> > > > >> > >>
> > > > >> > >> The idea is that those who need the extension features will
> > still
> > > > >> have a
> > > > >> > >> very clear way to get the implementation that provides these,
> > > > without
> > > > >> > ever
> > > > >> > >> having to cast.
> > > > >> > >>
> > > > >> > >> -- Rob
> > > > >> > >>
> > > > >> > >>
> > > > >> > >>
> > > > >> > >>
> > > > >> > >>
> > > > >> > >> On 24 January 2013 20:03, Rafael Schloming <rhs@alum.mit.edu
> >
> > > > wrote:
> > > > >> > >>
> > > > >> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
> > > > >> rob.j.godfrey@gmail.com
> > > > >> > >> > >wrote:
> > > > >> > >> >
> > > > >> > >> > > On 23 January 2013 17:36, Phil Harvey <
> > > > phil@philharveyonline.com>
> > > > >> > >> wrote:
> > > > >> > >> > > >
> > > > >> > >> > > > As part of the Proton JNI work, I would like to remove
> > all
> > > > calls
> > > > >> > to
> > > > >> > >> > > > proton-j implementation constructors from "client
> code".
> > I
> > > > >> intend
> > > > >> > >> that
> > > > >> > >> > > > factories will be used instead [1], thereby abstracting
> > > away
> > > > >> > whether
> > > > >> > >> > the
> > > > >> > >> > > > implementation is pure Java or proton-c-via-JNI.
> > > > >> > >> > > >
> > > > >> > >> > > > I'd like to check that folks are happy with me making
> > this
> > > > >> change,
> > > > >> > >> and
> > > > >> > >> > to
> > > > >> > >> > > > mention a couple of problems I've had.
> > > > >> > >> > > >
> > > > >> > >> > > > In this context, "client code" is anything outside the
> > > > current
> > > > >> > >> > > > sub-component, where our sub-components are Engine,
> > Codec,
> > > > >> Driver,
> > > > >> > >> > > Message
> > > > >> > >> > > > and Messenger, plus each of the contrib modules, and of
> > > > course
> > > > >> > third
> > > > >> > >> > > party
> > > > >> > >> > > > code.
> > > > >> > >> > > >
> > > > >> > >> > > > To enforce this abstraction, I am planning to make the
> > > > >> > constructors
> > > > >> > >> of
> > > > >> > >> > > the
> > > > >> > >> > > > affected classes package-private where possible. I
> > believe
> > > > >> that,
> > > > >> > >> > > although
> > > > >> > >> > > > third party projects might already be calling these
> > > > >> constructors,
> > > > >> > it
> > > > >> > >> is
> > > > >> > >> > > > acceptable for us to change its public API in this
> manner
> > > > while
> > > > >> > >> Proton
> > > > >> > >> > is
> > > > >> > >> > > > such a young project.
> > > > >> > >> > > >
> > > > >> > >> > >
> > > > >> > >> > > +1 to all of the above
> > > > >> > >> > >
> > > > >> > >> > > >
> > > > >> > >> > > > Please shout if you disagree with any of the above.
> > > > >> > >> > > >
> > > > >> > >> > > > Now, onto my problem. I started off with the
> > > > >> > >> > > org.apache.qpid.proton.engine.
> > > > >> > >> > > > impl package, and found that
> > > > >> > o.a.q.p.hawtdispatch.impl.AmqpTransport
> > > > >> > >> > > calls
> > > > >> > >> > > > various methods on ConnectionImpl and TransportImpl, so
> > > > simply
> > > > >> > using
> > > > >> > >> a
> > > > >> > >> > > > Connection and Transport will not work. I don't know
> > what
> > > > to do
> > > > >> > >> about
> > > > >> > >> > > > this, and would welcome people's opinions.
> > > > >> > >> > > >
> > > > >> > >> > >
> > > > >> > >> > > So, the simplest change would be to change the factories
> to
> > > use
> > > > >> > >> > > covariant return types
> > > > >> > >> > >
> > > > >> > >> > > e.g. EngingFactoryImpl becomes:
> > > > >> > >> > >
> > > > >> > >> > > @Override
> > > > >> > >> > > public ConnectionImpl createConnection()
> > > > >> > >> > > {
> > > > >> > >> > > return new ConnectionImpl();
> > > > >> > >> > > }
> > > > >> > >> > >
> > > > >> > >> > > @Override
> > > > >> > >> > > public TransportImpl createTransport()
> > > > >> > >> > > {
> > > > >> > >> > > return new TransportImpl();
> > > > >> > >> > > }
> > > > >> > >> > >
> > > > >> > >> > >
> > > > >> > >> > > ... etc
> > > > >> > >> > >
> > > > >> > >> > > Code that requires the extended functionality offered by
> > the
> > > > pure
> > > > >> > Java
> > > > >> > >> > > implementation can thus instantiate the desired Factory
> > > > directly.
> > > > >> > >> > >
> > > > >> > >> >
> > > > >> > >> > What's the point of going through the factory in this
> > scenario
> > > > >> rather
> > > > >> > >> than
> > > > >> > >> > directly instantiating the classes as Hiram suggests? Is
> > there
> > > > some
> > > > >> > class
> > > > >> > >> > of thing the factory would/could do that the constructor
> > > > >> > can't/shouldn't?
> > > > >> > >> >
> > > > >> > >> > A second refinement might be to actually separate out the
> > > > interface
> > > > >> > >> > > and implementation within the pure Java implementation so
> > > that
> > > > we
> > > > >> > have
> > > > >> > >> > > a well defined "extended" Java API. This interface could
> > > then
> > > > be
> > > > >> > the
> > > > >> > >> > > return type of the factory.
> > > > >> > >> > >
> > > > >> > >> >
> > > > >> > >> > Maybe I'm misunderstanding, but what's the point of using
> an
> > > > >> interface
> > > > >> > >> here
> > > > >> > >> > if you're still locked into the pure Java impl? Are you
> > > > expecting to
> > > > >> > swap
> > > > >> > >> > out that impl under some circumstances?
> > > > >> > >> >
> > > > >> > >> > --Rafael
> > > > >> > >> >
> > > > >> > >>
> > > > >> > >
> > > > >> > >
> > > > >> > >
> > > > >> > > --
> > > > >> > >
> > > > >> > > **
> > > > >> > >
> > > > >> > > *Hiram Chirino*
> > > > >> > >
> > > > >> > > *Engineering | Red Hat, Inc.*
> > > > >> > >
> > > > >> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
> > > > >> redhat.com
> > > > >> > *
> > > > >> > >
> > > > >> > > *skype: hiramchirino | twitter: @hiramchirino<
> > > > >> > http://twitter.com/hiramchirino>
> > > > >> > > *
> > > > >> > >
> > > > >> > > *blog: Hiram Chirino's Bit Mojo <
> http://hiramchirino.com/blog/
> > >*
> > > > >> >
> > > > >>
> > > >
> > >
> > >
> > >
> > > --
> > >
> > > **
> > >
> > > *Hiram Chirino*
> > >
> > > *Engineering | Red Hat, Inc.*
> > >
> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
> redhat.com
> > *
> > >
> > > *skype: hiramchirino | twitter: @hiramchirino<
> > > http://twitter.com/hiramchirino>
> > > *
> > >
> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
> > >
> >
>
--
**
*Hiram Chirino*
*Engineering | Red Hat, Inc.*
*hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com*
*skype: hiramchirino | twitter: @hiramchirino<http://twitter.com/hiramchirino>
*
*blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
Re: Reducing the visibility of proton-j constructors
Posted by Phil Harvey <ph...@philharveyonline.com>.
Yeah, I don't have a problem with doing this for Proton 0.4 if people think
the API change caused by our constructor visibility reduction is too
abrupt.
I don't think this is strictly necessary for Hiram's code to continue
working, since Keith and I are modifying it to use factories. This is
already done on the JNI branch - take a look at
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/contrib.
However, I understand there may be other projects already calling these
constructors so accept we need to be polite to our users.
We'd put @SuppressWarnings("deprecation") in EngineFactoryImpl so that its
(valid) use of the constructors wouldn't cause compiler warnings.
We would then make the constructors package-private in the followong
release - Proton 0.5.
Any objections?
Phil
On 28 January 2013 11:40, Rafael Schloming <rh...@alum.mit.edu> wrote:
> Would it be better if we were to deprecate the constructors and remove them
> in a future release?
>
> --Rafael
>
> On Fri, Jan 25, 2013 at 6:25 PM, Hiram Chirino <hiram@hiramchirino.com
> >wrote:
>
> > I was hoping I could upgrade to newer proton versions without having to
> > change my client code, since I was not planning on using the JNI impl
> > anyways.
> >
> >
> > On Fri, Jan 25, 2013 at 4:24 PM, Rajith Attapattu <rajith77@gmail.com
> > >wrote:
> >
> > > This is also my impression about Hirams work.
> > > So I'm not sure why there is resistance to the changes being proposed
> > > as it's only going to benefit in the long run should the impl changes.
> > > (I do understand that there will be some initial work in changing the
> > > code to use the factories and interfaces).
> > >
> > > I also thought part of Phil's (and Rob's) work was to come up with a
> > > set of interfaces for the API.
> > > If I'm not mistaken this work is already sitting in a branch ? (I
> > > stand to be corrected though).
> > >
> > > Rajith
> > >
> > > On Fri, Jan 25, 2013 at 3:44 PM, Rafael Schloming <rh...@alum.mit.edu>
> > > wrote:
> > > > FWIW, I don't think Hiram's usage fundamentally needs to depend on
> the
> > > pure
> > > > Java impl. I may be out of date on this one, but I believe his access
> > of
> > > > the implementation was done for expediency. I know at least in a
> number
> > > of
> > > > cases we discussed how the C implementation could accommodate his
> > > > requirements, and we plan do to so at some point. Perhaps our efforts
> > > here
> > > > would be better spent in building out the interface so that he
> doesn't
> > > need
> > > > direct access to the implementation.
> > > >
> > > > --Rafael
> > > >
> > > > On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <
> > phil@philharveyonline.com
> > > >wrote:
> > > >
> > > >> This has implications for API versioning,
> > > >>
> > > >> I think developers of third party code can assume that the API of
> > public
> > > >> methods, including constructors, will remain stable across minor
> > > revisions
> > > >> of a library. If a constructor is public then we, the Proton
> > > developers,
> > > >> have an obligation to respect this assumption (or at least to add
> "for
> > > >> internal use only" to its Javadoc).
> > > >>
> > > >> On reflection, I'm coming round to the idea that keeping these
> > > constructors
> > > >> public will place unreasonable restrictions on our ability to evolve
> > the
> > > >> implementation whilst keeping the API stable.
> > > >>
> > > >> Phil
> > > >>
> > > >>
> > > >> On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com>
> > wrote:
> > > >>
> > > >> > What's your need for the direct constructor?
> > > >> >
> > > >> >
> > > >> > -- Rob
> > > >> >
> > > >> > On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com>
> > > wrote:
> > > >> > > Let me clarify, I have no problem with adding Factories and
> using
> > > them
> > > >> > > everywhere possible. Just don't take my access away from direct
> > > >> > > constructors.
> > > >> > >
> > > >> > >
> > > >> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <
> > > rob.j.godfrey@gmail.com
> > > >> > >wrote:
> > > >> > >
> > > >> > >> In general dependency on implementation is bad, dependency on
> > > >> interface
> > > >> > is
> > > >> > >> better. One of the problems we've had with Qpid historically
> is
> > it
> > > >> > become
> > > >> > >> hard to test as we have dependencies on implementation
> > everywhere.
> > > >> > >>
> > > >> > >> From an end user perspective the difference is only replacing
> > > >> > >>
> > > >> > >> X x = new X();
> > > >> > >>
> > > >> > >> with
> > > >> > >>
> > > >> > >> X x = XFactory.newInstance();
> > > >> > >>
> > > >> > >> But it makes it a lot easier to properly describe the
> > functionality
> > > >> > being
> > > >> > >> returned. For implementation reasons, the concrete class may
> have
> > > >> public
> > > >> > >> methods that are not intended to be exposed to the application
> > > >> > programmer
> > > >> > >> but only to other cooperating classes. To avoid confusion, and
> > to
> > > >> make
> > > >> > it
> > > >> > >> clear to us the delta between the "shared" API and the API
> > > extensions
> > > >> > that
> > > >> > >> we are supporting for the pure Java implementation the
> interfaces
> > > seem
> > > >> > like
> > > >> > >> the right way to go for me.
> > > >> > >>
> > > >> > >> The idea is that those who need the extension features will
> still
> > > >> have a
> > > >> > >> very clear way to get the implementation that provides these,
> > > without
> > > >> > ever
> > > >> > >> having to cast.
> > > >> > >>
> > > >> > >> -- Rob
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu>
> > > wrote:
> > > >> > >>
> > > >> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
> > > >> rob.j.godfrey@gmail.com
> > > >> > >> > >wrote:
> > > >> > >> >
> > > >> > >> > > On 23 January 2013 17:36, Phil Harvey <
> > > phil@philharveyonline.com>
> > > >> > >> wrote:
> > > >> > >> > > >
> > > >> > >> > > > As part of the Proton JNI work, I would like to remove
> all
> > > calls
> > > >> > to
> > > >> > >> > > > proton-j implementation constructors from "client code".
> I
> > > >> intend
> > > >> > >> that
> > > >> > >> > > > factories will be used instead [1], thereby abstracting
> > away
> > > >> > whether
> > > >> > >> > the
> > > >> > >> > > > implementation is pure Java or proton-c-via-JNI.
> > > >> > >> > > >
> > > >> > >> > > > I'd like to check that folks are happy with me making
> this
> > > >> change,
> > > >> > >> and
> > > >> > >> > to
> > > >> > >> > > > mention a couple of problems I've had.
> > > >> > >> > > >
> > > >> > >> > > > In this context, "client code" is anything outside the
> > > current
> > > >> > >> > > > sub-component, where our sub-components are Engine,
> Codec,
> > > >> Driver,
> > > >> > >> > > Message
> > > >> > >> > > > and Messenger, plus each of the contrib modules, and of
> > > course
> > > >> > third
> > > >> > >> > > party
> > > >> > >> > > > code.
> > > >> > >> > > >
> > > >> > >> > > > To enforce this abstraction, I am planning to make the
> > > >> > constructors
> > > >> > >> of
> > > >> > >> > > the
> > > >> > >> > > > affected classes package-private where possible. I
> believe
> > > >> that,
> > > >> > >> > > although
> > > >> > >> > > > third party projects might already be calling these
> > > >> constructors,
> > > >> > it
> > > >> > >> is
> > > >> > >> > > > acceptable for us to change its public API in this manner
> > > while
> > > >> > >> Proton
> > > >> > >> > is
> > > >> > >> > > > such a young project.
> > > >> > >> > > >
> > > >> > >> > >
> > > >> > >> > > +1 to all of the above
> > > >> > >> > >
> > > >> > >> > > >
> > > >> > >> > > > Please shout if you disagree with any of the above.
> > > >> > >> > > >
> > > >> > >> > > > Now, onto my problem. I started off with the
> > > >> > >> > > org.apache.qpid.proton.engine.
> > > >> > >> > > > impl package, and found that
> > > >> > o.a.q.p.hawtdispatch.impl.AmqpTransport
> > > >> > >> > > calls
> > > >> > >> > > > various methods on ConnectionImpl and TransportImpl, so
> > > simply
> > > >> > using
> > > >> > >> a
> > > >> > >> > > > Connection and Transport will not work. I don't know
> what
> > > to do
> > > >> > >> about
> > > >> > >> > > > this, and would welcome people's opinions.
> > > >> > >> > > >
> > > >> > >> > >
> > > >> > >> > > So, the simplest change would be to change the factories to
> > use
> > > >> > >> > > covariant return types
> > > >> > >> > >
> > > >> > >> > > e.g. EngingFactoryImpl becomes:
> > > >> > >> > >
> > > >> > >> > > @Override
> > > >> > >> > > public ConnectionImpl createConnection()
> > > >> > >> > > {
> > > >> > >> > > return new ConnectionImpl();
> > > >> > >> > > }
> > > >> > >> > >
> > > >> > >> > > @Override
> > > >> > >> > > public TransportImpl createTransport()
> > > >> > >> > > {
> > > >> > >> > > return new TransportImpl();
> > > >> > >> > > }
> > > >> > >> > >
> > > >> > >> > >
> > > >> > >> > > ... etc
> > > >> > >> > >
> > > >> > >> > > Code that requires the extended functionality offered by
> the
> > > pure
> > > >> > Java
> > > >> > >> > > implementation can thus instantiate the desired Factory
> > > directly.
> > > >> > >> > >
> > > >> > >> >
> > > >> > >> > What's the point of going through the factory in this
> scenario
> > > >> rather
> > > >> > >> than
> > > >> > >> > directly instantiating the classes as Hiram suggests? Is
> there
> > > some
> > > >> > class
> > > >> > >> > of thing the factory would/could do that the constructor
> > > >> > can't/shouldn't?
> > > >> > >> >
> > > >> > >> > A second refinement might be to actually separate out the
> > > interface
> > > >> > >> > > and implementation within the pure Java implementation so
> > that
> > > we
> > > >> > have
> > > >> > >> > > a well defined "extended" Java API. This interface could
> > then
> > > be
> > > >> > the
> > > >> > >> > > return type of the factory.
> > > >> > >> > >
> > > >> > >> >
> > > >> > >> > Maybe I'm misunderstanding, but what's the point of using an
> > > >> interface
> > > >> > >> here
> > > >> > >> > if you're still locked into the pure Java impl? Are you
> > > expecting to
> > > >> > swap
> > > >> > >> > out that impl under some circumstances?
> > > >> > >> >
> > > >> > >> > --Rafael
> > > >> > >> >
> > > >> > >>
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > >
> > > >> > > **
> > > >> > >
> > > >> > > *Hiram Chirino*
> > > >> > >
> > > >> > > *Engineering | Red Hat, Inc.*
> > > >> > >
> > > >> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
> > > >> redhat.com
> > > >> > *
> > > >> > >
> > > >> > > *skype: hiramchirino | twitter: @hiramchirino<
> > > >> > http://twitter.com/hiramchirino>
> > > >> > > *
> > > >> > >
> > > >> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/
> >*
> > > >> >
> > > >>
> > >
> >
> >
> >
> > --
> >
> > **
> >
> > *Hiram Chirino*
> >
> > *Engineering | Red Hat, Inc.*
> >
> > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com
> *
> >
> > *skype: hiramchirino | twitter: @hiramchirino<
> > http://twitter.com/hiramchirino>
> > *
> >
> > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
> >
>
Re: Reducing the visibility of proton-j constructors
Posted by Hiram Chirino <hi...@hiramchirino.com>.
Yeah,
At least it will give me time to migrate over and report any issues I may
run into.
On Mon, Jan 28, 2013 at 6:40 AM, Rafael Schloming <rh...@alum.mit.edu> wrote:
> Would it be better if we were to deprecate the constructors and remove them
> in a future release?
>
> --Rafael
>
> On Fri, Jan 25, 2013 at 6:25 PM, Hiram Chirino <hiram@hiramchirino.com
> >wrote:
>
> > I was hoping I could upgrade to newer proton versions without having to
> > change my client code, since I was not planning on using the JNI impl
> > anyways.
> >
> >
> > On Fri, Jan 25, 2013 at 4:24 PM, Rajith Attapattu <rajith77@gmail.com
> > >wrote:
> >
> > > This is also my impression about Hirams work.
> > > So I'm not sure why there is resistance to the changes being proposed
> > > as it's only going to benefit in the long run should the impl changes.
> > > (I do understand that there will be some initial work in changing the
> > > code to use the factories and interfaces).
> > >
> > > I also thought part of Phil's (and Rob's) work was to come up with a
> > > set of interfaces for the API.
> > > If I'm not mistaken this work is already sitting in a branch ? (I
> > > stand to be corrected though).
> > >
> > > Rajith
> > >
> > > On Fri, Jan 25, 2013 at 3:44 PM, Rafael Schloming <rh...@alum.mit.edu>
> > > wrote:
> > > > FWIW, I don't think Hiram's usage fundamentally needs to depend on
> the
> > > pure
> > > > Java impl. I may be out of date on this one, but I believe his access
> > of
> > > > the implementation was done for expediency. I know at least in a
> number
> > > of
> > > > cases we discussed how the C implementation could accommodate his
> > > > requirements, and we plan do to so at some point. Perhaps our efforts
> > > here
> > > > would be better spent in building out the interface so that he
> doesn't
> > > need
> > > > direct access to the implementation.
> > > >
> > > > --Rafael
> > > >
> > > > On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <
> > phil@philharveyonline.com
> > > >wrote:
> > > >
> > > >> This has implications for API versioning,
> > > >>
> > > >> I think developers of third party code can assume that the API of
> > public
> > > >> methods, including constructors, will remain stable across minor
> > > revisions
> > > >> of a library. If a constructor is public then we, the Proton
> > > developers,
> > > >> have an obligation to respect this assumption (or at least to add
> "for
> > > >> internal use only" to its Javadoc).
> > > >>
> > > >> On reflection, I'm coming round to the idea that keeping these
> > > constructors
> > > >> public will place unreasonable restrictions on our ability to evolve
> > the
> > > >> implementation whilst keeping the API stable.
> > > >>
> > > >> Phil
> > > >>
> > > >>
> > > >> On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com>
> > wrote:
> > > >>
> > > >> > What's your need for the direct constructor?
> > > >> >
> > > >> >
> > > >> > -- Rob
> > > >> >
> > > >> > On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com>
> > > wrote:
> > > >> > > Let me clarify, I have no problem with adding Factories and
> using
> > > them
> > > >> > > everywhere possible. Just don't take my access away from direct
> > > >> > > constructors.
> > > >> > >
> > > >> > >
> > > >> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <
> > > rob.j.godfrey@gmail.com
> > > >> > >wrote:
> > > >> > >
> > > >> > >> In general dependency on implementation is bad, dependency on
> > > >> interface
> > > >> > is
> > > >> > >> better. One of the problems we've had with Qpid historically
> is
> > it
> > > >> > become
> > > >> > >> hard to test as we have dependencies on implementation
> > everywhere.
> > > >> > >>
> > > >> > >> From an end user perspective the difference is only replacing
> > > >> > >>
> > > >> > >> X x = new X();
> > > >> > >>
> > > >> > >> with
> > > >> > >>
> > > >> > >> X x = XFactory.newInstance();
> > > >> > >>
> > > >> > >> But it makes it a lot easier to properly describe the
> > functionality
> > > >> > being
> > > >> > >> returned. For implementation reasons, the concrete class may
> have
> > > >> public
> > > >> > >> methods that are not intended to be exposed to the application
> > > >> > programmer
> > > >> > >> but only to other cooperating classes. To avoid confusion, and
> > to
> > > >> make
> > > >> > it
> > > >> > >> clear to us the delta between the "shared" API and the API
> > > extensions
> > > >> > that
> > > >> > >> we are supporting for the pure Java implementation the
> interfaces
> > > seem
> > > >> > like
> > > >> > >> the right way to go for me.
> > > >> > >>
> > > >> > >> The idea is that those who need the extension features will
> still
> > > >> have a
> > > >> > >> very clear way to get the implementation that provides these,
> > > without
> > > >> > ever
> > > >> > >> having to cast.
> > > >> > >>
> > > >> > >> -- Rob
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu>
> > > wrote:
> > > >> > >>
> > > >> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
> > > >> rob.j.godfrey@gmail.com
> > > >> > >> > >wrote:
> > > >> > >> >
> > > >> > >> > > On 23 January 2013 17:36, Phil Harvey <
> > > phil@philharveyonline.com>
> > > >> > >> wrote:
> > > >> > >> > > >
> > > >> > >> > > > As part of the Proton JNI work, I would like to remove
> all
> > > calls
> > > >> > to
> > > >> > >> > > > proton-j implementation constructors from "client code".
> I
> > > >> intend
> > > >> > >> that
> > > >> > >> > > > factories will be used instead [1], thereby abstracting
> > away
> > > >> > whether
> > > >> > >> > the
> > > >> > >> > > > implementation is pure Java or proton-c-via-JNI.
> > > >> > >> > > >
> > > >> > >> > > > I'd like to check that folks are happy with me making
> this
> > > >> change,
> > > >> > >> and
> > > >> > >> > to
> > > >> > >> > > > mention a couple of problems I've had.
> > > >> > >> > > >
> > > >> > >> > > > In this context, "client code" is anything outside the
> > > current
> > > >> > >> > > > sub-component, where our sub-components are Engine,
> Codec,
> > > >> Driver,
> > > >> > >> > > Message
> > > >> > >> > > > and Messenger, plus each of the contrib modules, and of
> > > course
> > > >> > third
> > > >> > >> > > party
> > > >> > >> > > > code.
> > > >> > >> > > >
> > > >> > >> > > > To enforce this abstraction, I am planning to make the
> > > >> > constructors
> > > >> > >> of
> > > >> > >> > > the
> > > >> > >> > > > affected classes package-private where possible. I
> believe
> > > >> that,
> > > >> > >> > > although
> > > >> > >> > > > third party projects might already be calling these
> > > >> constructors,
> > > >> > it
> > > >> > >> is
> > > >> > >> > > > acceptable for us to change its public API in this manner
> > > while
> > > >> > >> Proton
> > > >> > >> > is
> > > >> > >> > > > such a young project.
> > > >> > >> > > >
> > > >> > >> > >
> > > >> > >> > > +1 to all of the above
> > > >> > >> > >
> > > >> > >> > > >
> > > >> > >> > > > Please shout if you disagree with any of the above.
> > > >> > >> > > >
> > > >> > >> > > > Now, onto my problem. I started off with the
> > > >> > >> > > org.apache.qpid.proton.engine.
> > > >> > >> > > > impl package, and found that
> > > >> > o.a.q.p.hawtdispatch.impl.AmqpTransport
> > > >> > >> > > calls
> > > >> > >> > > > various methods on ConnectionImpl and TransportImpl, so
> > > simply
> > > >> > using
> > > >> > >> a
> > > >> > >> > > > Connection and Transport will not work. I don't know
> what
> > > to do
> > > >> > >> about
> > > >> > >> > > > this, and would welcome people's opinions.
> > > >> > >> > > >
> > > >> > >> > >
> > > >> > >> > > So, the simplest change would be to change the factories to
> > use
> > > >> > >> > > covariant return types
> > > >> > >> > >
> > > >> > >> > > e.g. EngingFactoryImpl becomes:
> > > >> > >> > >
> > > >> > >> > > @Override
> > > >> > >> > > public ConnectionImpl createConnection()
> > > >> > >> > > {
> > > >> > >> > > return new ConnectionImpl();
> > > >> > >> > > }
> > > >> > >> > >
> > > >> > >> > > @Override
> > > >> > >> > > public TransportImpl createTransport()
> > > >> > >> > > {
> > > >> > >> > > return new TransportImpl();
> > > >> > >> > > }
> > > >> > >> > >
> > > >> > >> > >
> > > >> > >> > > ... etc
> > > >> > >> > >
> > > >> > >> > > Code that requires the extended functionality offered by
> the
> > > pure
> > > >> > Java
> > > >> > >> > > implementation can thus instantiate the desired Factory
> > > directly.
> > > >> > >> > >
> > > >> > >> >
> > > >> > >> > What's the point of going through the factory in this
> scenario
> > > >> rather
> > > >> > >> than
> > > >> > >> > directly instantiating the classes as Hiram suggests? Is
> there
> > > some
> > > >> > class
> > > >> > >> > of thing the factory would/could do that the constructor
> > > >> > can't/shouldn't?
> > > >> > >> >
> > > >> > >> > A second refinement might be to actually separate out the
> > > interface
> > > >> > >> > > and implementation within the pure Java implementation so
> > that
> > > we
> > > >> > have
> > > >> > >> > > a well defined "extended" Java API. This interface could
> > then
> > > be
> > > >> > the
> > > >> > >> > > return type of the factory.
> > > >> > >> > >
> > > >> > >> >
> > > >> > >> > Maybe I'm misunderstanding, but what's the point of using an
> > > >> interface
> > > >> > >> here
> > > >> > >> > if you're still locked into the pure Java impl? Are you
> > > expecting to
> > > >> > swap
> > > >> > >> > out that impl under some circumstances?
> > > >> > >> >
> > > >> > >> > --Rafael
> > > >> > >> >
> > > >> > >>
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > >
> > > >> > > **
> > > >> > >
> > > >> > > *Hiram Chirino*
> > > >> > >
> > > >> > > *Engineering | Red Hat, Inc.*
> > > >> > >
> > > >> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
> > > >> redhat.com
> > > >> > *
> > > >> > >
> > > >> > > *skype: hiramchirino | twitter: @hiramchirino<
> > > >> > http://twitter.com/hiramchirino>
> > > >> > > *
> > > >> > >
> > > >> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/
> >*
> > > >> >
> > > >>
> > >
> >
> >
> >
> > --
> >
> > **
> >
> > *Hiram Chirino*
> >
> > *Engineering | Red Hat, Inc.*
> >
> > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com
> *
> >
> > *skype: hiramchirino | twitter: @hiramchirino<
> > http://twitter.com/hiramchirino>
> > *
> >
> > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
> >
>
--
**
*Hiram Chirino*
*Engineering | Red Hat, Inc.*
*hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com*
*skype: hiramchirino | twitter: @hiramchirino<http://twitter.com/hiramchirino>
*
*blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
Re: Reducing the visibility of proton-j constructors
Posted by Rafael Schloming <rh...@alum.mit.edu>.
Would it be better if we were to deprecate the constructors and remove them
in a future release?
--Rafael
On Fri, Jan 25, 2013 at 6:25 PM, Hiram Chirino <hi...@hiramchirino.com>wrote:
> I was hoping I could upgrade to newer proton versions without having to
> change my client code, since I was not planning on using the JNI impl
> anyways.
>
>
> On Fri, Jan 25, 2013 at 4:24 PM, Rajith Attapattu <rajith77@gmail.com
> >wrote:
>
> > This is also my impression about Hirams work.
> > So I'm not sure why there is resistance to the changes being proposed
> > as it's only going to benefit in the long run should the impl changes.
> > (I do understand that there will be some initial work in changing the
> > code to use the factories and interfaces).
> >
> > I also thought part of Phil's (and Rob's) work was to come up with a
> > set of interfaces for the API.
> > If I'm not mistaken this work is already sitting in a branch ? (I
> > stand to be corrected though).
> >
> > Rajith
> >
> > On Fri, Jan 25, 2013 at 3:44 PM, Rafael Schloming <rh...@alum.mit.edu>
> > wrote:
> > > FWIW, I don't think Hiram's usage fundamentally needs to depend on the
> > pure
> > > Java impl. I may be out of date on this one, but I believe his access
> of
> > > the implementation was done for expediency. I know at least in a number
> > of
> > > cases we discussed how the C implementation could accommodate his
> > > requirements, and we plan do to so at some point. Perhaps our efforts
> > here
> > > would be better spent in building out the interface so that he doesn't
> > need
> > > direct access to the implementation.
> > >
> > > --Rafael
> > >
> > > On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <
> phil@philharveyonline.com
> > >wrote:
> > >
> > >> This has implications for API versioning,
> > >>
> > >> I think developers of third party code can assume that the API of
> public
> > >> methods, including constructors, will remain stable across minor
> > revisions
> > >> of a library. If a constructor is public then we, the Proton
> > developers,
> > >> have an obligation to respect this assumption (or at least to add "for
> > >> internal use only" to its Javadoc).
> > >>
> > >> On reflection, I'm coming round to the idea that keeping these
> > constructors
> > >> public will place unreasonable restrictions on our ability to evolve
> the
> > >> implementation whilst keeping the API stable.
> > >>
> > >> Phil
> > >>
> > >>
> > >> On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com>
> wrote:
> > >>
> > >> > What's your need for the direct constructor?
> > >> >
> > >> >
> > >> > -- Rob
> > >> >
> > >> > On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com>
> > wrote:
> > >> > > Let me clarify, I have no problem with adding Factories and using
> > them
> > >> > > everywhere possible. Just don't take my access away from direct
> > >> > > constructors.
> > >> > >
> > >> > >
> > >> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <
> > rob.j.godfrey@gmail.com
> > >> > >wrote:
> > >> > >
> > >> > >> In general dependency on implementation is bad, dependency on
> > >> interface
> > >> > is
> > >> > >> better. One of the problems we've had with Qpid historically is
> it
> > >> > become
> > >> > >> hard to test as we have dependencies on implementation
> everywhere.
> > >> > >>
> > >> > >> From an end user perspective the difference is only replacing
> > >> > >>
> > >> > >> X x = new X();
> > >> > >>
> > >> > >> with
> > >> > >>
> > >> > >> X x = XFactory.newInstance();
> > >> > >>
> > >> > >> But it makes it a lot easier to properly describe the
> functionality
> > >> > being
> > >> > >> returned. For implementation reasons, the concrete class may have
> > >> public
> > >> > >> methods that are not intended to be exposed to the application
> > >> > programmer
> > >> > >> but only to other cooperating classes. To avoid confusion, and
> to
> > >> make
> > >> > it
> > >> > >> clear to us the delta between the "shared" API and the API
> > extensions
> > >> > that
> > >> > >> we are supporting for the pure Java implementation the interfaces
> > seem
> > >> > like
> > >> > >> the right way to go for me.
> > >> > >>
> > >> > >> The idea is that those who need the extension features will still
> > >> have a
> > >> > >> very clear way to get the implementation that provides these,
> > without
> > >> > ever
> > >> > >> having to cast.
> > >> > >>
> > >> > >> -- Rob
> > >> > >>
> > >> > >>
> > >> > >>
> > >> > >>
> > >> > >>
> > >> > >> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu>
> > wrote:
> > >> > >>
> > >> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
> > >> rob.j.godfrey@gmail.com
> > >> > >> > >wrote:
> > >> > >> >
> > >> > >> > > On 23 January 2013 17:36, Phil Harvey <
> > phil@philharveyonline.com>
> > >> > >> wrote:
> > >> > >> > > >
> > >> > >> > > > As part of the Proton JNI work, I would like to remove all
> > calls
> > >> > to
> > >> > >> > > > proton-j implementation constructors from "client code". I
> > >> intend
> > >> > >> that
> > >> > >> > > > factories will be used instead [1], thereby abstracting
> away
> > >> > whether
> > >> > >> > the
> > >> > >> > > > implementation is pure Java or proton-c-via-JNI.
> > >> > >> > > >
> > >> > >> > > > I'd like to check that folks are happy with me making this
> > >> change,
> > >> > >> and
> > >> > >> > to
> > >> > >> > > > mention a couple of problems I've had.
> > >> > >> > > >
> > >> > >> > > > In this context, "client code" is anything outside the
> > current
> > >> > >> > > > sub-component, where our sub-components are Engine, Codec,
> > >> Driver,
> > >> > >> > > Message
> > >> > >> > > > and Messenger, plus each of the contrib modules, and of
> > course
> > >> > third
> > >> > >> > > party
> > >> > >> > > > code.
> > >> > >> > > >
> > >> > >> > > > To enforce this abstraction, I am planning to make the
> > >> > constructors
> > >> > >> of
> > >> > >> > > the
> > >> > >> > > > affected classes package-private where possible. I believe
> > >> that,
> > >> > >> > > although
> > >> > >> > > > third party projects might already be calling these
> > >> constructors,
> > >> > it
> > >> > >> is
> > >> > >> > > > acceptable for us to change its public API in this manner
> > while
> > >> > >> Proton
> > >> > >> > is
> > >> > >> > > > such a young project.
> > >> > >> > > >
> > >> > >> > >
> > >> > >> > > +1 to all of the above
> > >> > >> > >
> > >> > >> > > >
> > >> > >> > > > Please shout if you disagree with any of the above.
> > >> > >> > > >
> > >> > >> > > > Now, onto my problem. I started off with the
> > >> > >> > > org.apache.qpid.proton.engine.
> > >> > >> > > > impl package, and found that
> > >> > o.a.q.p.hawtdispatch.impl.AmqpTransport
> > >> > >> > > calls
> > >> > >> > > > various methods on ConnectionImpl and TransportImpl, so
> > simply
> > >> > using
> > >> > >> a
> > >> > >> > > > Connection and Transport will not work. I don't know what
> > to do
> > >> > >> about
> > >> > >> > > > this, and would welcome people's opinions.
> > >> > >> > > >
> > >> > >> > >
> > >> > >> > > So, the simplest change would be to change the factories to
> use
> > >> > >> > > covariant return types
> > >> > >> > >
> > >> > >> > > e.g. EngingFactoryImpl becomes:
> > >> > >> > >
> > >> > >> > > @Override
> > >> > >> > > public ConnectionImpl createConnection()
> > >> > >> > > {
> > >> > >> > > return new ConnectionImpl();
> > >> > >> > > }
> > >> > >> > >
> > >> > >> > > @Override
> > >> > >> > > public TransportImpl createTransport()
> > >> > >> > > {
> > >> > >> > > return new TransportImpl();
> > >> > >> > > }
> > >> > >> > >
> > >> > >> > >
> > >> > >> > > ... etc
> > >> > >> > >
> > >> > >> > > Code that requires the extended functionality offered by the
> > pure
> > >> > Java
> > >> > >> > > implementation can thus instantiate the desired Factory
> > directly.
> > >> > >> > >
> > >> > >> >
> > >> > >> > What's the point of going through the factory in this scenario
> > >> rather
> > >> > >> than
> > >> > >> > directly instantiating the classes as Hiram suggests? Is there
> > some
> > >> > class
> > >> > >> > of thing the factory would/could do that the constructor
> > >> > can't/shouldn't?
> > >> > >> >
> > >> > >> > A second refinement might be to actually separate out the
> > interface
> > >> > >> > > and implementation within the pure Java implementation so
> that
> > we
> > >> > have
> > >> > >> > > a well defined "extended" Java API. This interface could
> then
> > be
> > >> > the
> > >> > >> > > return type of the factory.
> > >> > >> > >
> > >> > >> >
> > >> > >> > Maybe I'm misunderstanding, but what's the point of using an
> > >> interface
> > >> > >> here
> > >> > >> > if you're still locked into the pure Java impl? Are you
> > expecting to
> > >> > swap
> > >> > >> > out that impl under some circumstances?
> > >> > >> >
> > >> > >> > --Rafael
> > >> > >> >
> > >> > >>
> > >> > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > >
> > >> > > **
> > >> > >
> > >> > > *Hiram Chirino*
> > >> > >
> > >> > > *Engineering | Red Hat, Inc.*
> > >> > >
> > >> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
> > >> redhat.com
> > >> > *
> > >> > >
> > >> > > *skype: hiramchirino | twitter: @hiramchirino<
> > >> > http://twitter.com/hiramchirino>
> > >> > > *
> > >> > >
> > >> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
> > >> >
> > >>
> >
>
>
>
> --
>
> **
>
> *Hiram Chirino*
>
> *Engineering | Red Hat, Inc.*
>
> *hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com*
>
> *skype: hiramchirino | twitter: @hiramchirino<
> http://twitter.com/hiramchirino>
> *
>
> *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
>
Re: Reducing the visibility of proton-j constructors
Posted by Hiram Chirino <hi...@hiramchirino.com>.
I was hoping I could upgrade to newer proton versions without having to
change my client code, since I was not planning on using the JNI impl
anyways.
On Fri, Jan 25, 2013 at 4:24 PM, Rajith Attapattu <ra...@gmail.com>wrote:
> This is also my impression about Hirams work.
> So I'm not sure why there is resistance to the changes being proposed
> as it's only going to benefit in the long run should the impl changes.
> (I do understand that there will be some initial work in changing the
> code to use the factories and interfaces).
>
> I also thought part of Phil's (and Rob's) work was to come up with a
> set of interfaces for the API.
> If I'm not mistaken this work is already sitting in a branch ? (I
> stand to be corrected though).
>
> Rajith
>
> On Fri, Jan 25, 2013 at 3:44 PM, Rafael Schloming <rh...@alum.mit.edu>
> wrote:
> > FWIW, I don't think Hiram's usage fundamentally needs to depend on the
> pure
> > Java impl. I may be out of date on this one, but I believe his access of
> > the implementation was done for expediency. I know at least in a number
> of
> > cases we discussed how the C implementation could accommodate his
> > requirements, and we plan do to so at some point. Perhaps our efforts
> here
> > would be better spent in building out the interface so that he doesn't
> need
> > direct access to the implementation.
> >
> > --Rafael
> >
> > On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <phil@philharveyonline.com
> >wrote:
> >
> >> This has implications for API versioning,
> >>
> >> I think developers of third party code can assume that the API of public
> >> methods, including constructors, will remain stable across minor
> revisions
> >> of a library. If a constructor is public then we, the Proton
> developers,
> >> have an obligation to respect this assumption (or at least to add "for
> >> internal use only" to its Javadoc).
> >>
> >> On reflection, I'm coming round to the idea that keeping these
> constructors
> >> public will place unreasonable restrictions on our ability to evolve the
> >> implementation whilst keeping the API stable.
> >>
> >> Phil
> >>
> >>
> >> On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com> wrote:
> >>
> >> > What's your need for the direct constructor?
> >> >
> >> >
> >> > -- Rob
> >> >
> >> > On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com>
> wrote:
> >> > > Let me clarify, I have no problem with adding Factories and using
> them
> >> > > everywhere possible. Just don't take my access away from direct
> >> > > constructors.
> >> > >
> >> > >
> >> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <
> rob.j.godfrey@gmail.com
> >> > >wrote:
> >> > >
> >> > >> In general dependency on implementation is bad, dependency on
> >> interface
> >> > is
> >> > >> better. One of the problems we've had with Qpid historically is it
> >> > become
> >> > >> hard to test as we have dependencies on implementation everywhere.
> >> > >>
> >> > >> From an end user perspective the difference is only replacing
> >> > >>
> >> > >> X x = new X();
> >> > >>
> >> > >> with
> >> > >>
> >> > >> X x = XFactory.newInstance();
> >> > >>
> >> > >> But it makes it a lot easier to properly describe the functionality
> >> > being
> >> > >> returned. For implementation reasons, the concrete class may have
> >> public
> >> > >> methods that are not intended to be exposed to the application
> >> > programmer
> >> > >> but only to other cooperating classes. To avoid confusion, and to
> >> make
> >> > it
> >> > >> clear to us the delta between the "shared" API and the API
> extensions
> >> > that
> >> > >> we are supporting for the pure Java implementation the interfaces
> seem
> >> > like
> >> > >> the right way to go for me.
> >> > >>
> >> > >> The idea is that those who need the extension features will still
> >> have a
> >> > >> very clear way to get the implementation that provides these,
> without
> >> > ever
> >> > >> having to cast.
> >> > >>
> >> > >> -- Rob
> >> > >>
> >> > >>
> >> > >>
> >> > >>
> >> > >>
> >> > >> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu>
> wrote:
> >> > >>
> >> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
> >> rob.j.godfrey@gmail.com
> >> > >> > >wrote:
> >> > >> >
> >> > >> > > On 23 January 2013 17:36, Phil Harvey <
> phil@philharveyonline.com>
> >> > >> wrote:
> >> > >> > > >
> >> > >> > > > As part of the Proton JNI work, I would like to remove all
> calls
> >> > to
> >> > >> > > > proton-j implementation constructors from "client code". I
> >> intend
> >> > >> that
> >> > >> > > > factories will be used instead [1], thereby abstracting away
> >> > whether
> >> > >> > the
> >> > >> > > > implementation is pure Java or proton-c-via-JNI.
> >> > >> > > >
> >> > >> > > > I'd like to check that folks are happy with me making this
> >> change,
> >> > >> and
> >> > >> > to
> >> > >> > > > mention a couple of problems I've had.
> >> > >> > > >
> >> > >> > > > In this context, "client code" is anything outside the
> current
> >> > >> > > > sub-component, where our sub-components are Engine, Codec,
> >> Driver,
> >> > >> > > Message
> >> > >> > > > and Messenger, plus each of the contrib modules, and of
> course
> >> > third
> >> > >> > > party
> >> > >> > > > code.
> >> > >> > > >
> >> > >> > > > To enforce this abstraction, I am planning to make the
> >> > constructors
> >> > >> of
> >> > >> > > the
> >> > >> > > > affected classes package-private where possible. I believe
> >> that,
> >> > >> > > although
> >> > >> > > > third party projects might already be calling these
> >> constructors,
> >> > it
> >> > >> is
> >> > >> > > > acceptable for us to change its public API in this manner
> while
> >> > >> Proton
> >> > >> > is
> >> > >> > > > such a young project.
> >> > >> > > >
> >> > >> > >
> >> > >> > > +1 to all of the above
> >> > >> > >
> >> > >> > > >
> >> > >> > > > Please shout if you disagree with any of the above.
> >> > >> > > >
> >> > >> > > > Now, onto my problem. I started off with the
> >> > >> > > org.apache.qpid.proton.engine.
> >> > >> > > > impl package, and found that
> >> > o.a.q.p.hawtdispatch.impl.AmqpTransport
> >> > >> > > calls
> >> > >> > > > various methods on ConnectionImpl and TransportImpl, so
> simply
> >> > using
> >> > >> a
> >> > >> > > > Connection and Transport will not work. I don't know what
> to do
> >> > >> about
> >> > >> > > > this, and would welcome people's opinions.
> >> > >> > > >
> >> > >> > >
> >> > >> > > So, the simplest change would be to change the factories to use
> >> > >> > > covariant return types
> >> > >> > >
> >> > >> > > e.g. EngingFactoryImpl becomes:
> >> > >> > >
> >> > >> > > @Override
> >> > >> > > public ConnectionImpl createConnection()
> >> > >> > > {
> >> > >> > > return new ConnectionImpl();
> >> > >> > > }
> >> > >> > >
> >> > >> > > @Override
> >> > >> > > public TransportImpl createTransport()
> >> > >> > > {
> >> > >> > > return new TransportImpl();
> >> > >> > > }
> >> > >> > >
> >> > >> > >
> >> > >> > > ... etc
> >> > >> > >
> >> > >> > > Code that requires the extended functionality offered by the
> pure
> >> > Java
> >> > >> > > implementation can thus instantiate the desired Factory
> directly.
> >> > >> > >
> >> > >> >
> >> > >> > What's the point of going through the factory in this scenario
> >> rather
> >> > >> than
> >> > >> > directly instantiating the classes as Hiram suggests? Is there
> some
> >> > class
> >> > >> > of thing the factory would/could do that the constructor
> >> > can't/shouldn't?
> >> > >> >
> >> > >> > A second refinement might be to actually separate out the
> interface
> >> > >> > > and implementation within the pure Java implementation so that
> we
> >> > have
> >> > >> > > a well defined "extended" Java API. This interface could then
> be
> >> > the
> >> > >> > > return type of the factory.
> >> > >> > >
> >> > >> >
> >> > >> > Maybe I'm misunderstanding, but what's the point of using an
> >> interface
> >> > >> here
> >> > >> > if you're still locked into the pure Java impl? Are you
> expecting to
> >> > swap
> >> > >> > out that impl under some circumstances?
> >> > >> >
> >> > >> > --Rafael
> >> > >> >
> >> > >>
> >> > >
> >> > >
> >> > >
> >> > > --
> >> > >
> >> > > **
> >> > >
> >> > > *Hiram Chirino*
> >> > >
> >> > > *Engineering | Red Hat, Inc.*
> >> > >
> >> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
> >> redhat.com
> >> > *
> >> > >
> >> > > *skype: hiramchirino | twitter: @hiramchirino<
> >> > http://twitter.com/hiramchirino>
> >> > > *
> >> > >
> >> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
> >> >
> >>
>
--
**
*Hiram Chirino*
*Engineering | Red Hat, Inc.*
*hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com*
*skype: hiramchirino | twitter: @hiramchirino<http://twitter.com/hiramchirino>
*
*blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
Re: Reducing the visibility of proton-j constructors
Posted by Rajith Attapattu <ra...@gmail.com>.
This is also my impression about Hirams work.
So I'm not sure why there is resistance to the changes being proposed
as it's only going to benefit in the long run should the impl changes.
(I do understand that there will be some initial work in changing the
code to use the factories and interfaces).
I also thought part of Phil's (and Rob's) work was to come up with a
set of interfaces for the API.
If I'm not mistaken this work is already sitting in a branch ? (I
stand to be corrected though).
Rajith
On Fri, Jan 25, 2013 at 3:44 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:
> FWIW, I don't think Hiram's usage fundamentally needs to depend on the pure
> Java impl. I may be out of date on this one, but I believe his access of
> the implementation was done for expediency. I know at least in a number of
> cases we discussed how the C implementation could accommodate his
> requirements, and we plan do to so at some point. Perhaps our efforts here
> would be better spent in building out the interface so that he doesn't need
> direct access to the implementation.
>
> --Rafael
>
> On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <ph...@philharveyonline.com>wrote:
>
>> This has implications for API versioning,
>>
>> I think developers of third party code can assume that the API of public
>> methods, including constructors, will remain stable across minor revisions
>> of a library. If a constructor is public then we, the Proton developers,
>> have an obligation to respect this assumption (or at least to add "for
>> internal use only" to its Javadoc).
>>
>> On reflection, I'm coming round to the idea that keeping these constructors
>> public will place unreasonable restrictions on our ability to evolve the
>> implementation whilst keeping the API stable.
>>
>> Phil
>>
>>
>> On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com> wrote:
>>
>> > What's your need for the direct constructor?
>> >
>> >
>> > -- Rob
>> >
>> > On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com> wrote:
>> > > Let me clarify, I have no problem with adding Factories and using them
>> > > everywhere possible. Just don't take my access away from direct
>> > > constructors.
>> > >
>> > >
>> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <rob.j.godfrey@gmail.com
>> > >wrote:
>> > >
>> > >> In general dependency on implementation is bad, dependency on
>> interface
>> > is
>> > >> better. One of the problems we've had with Qpid historically is it
>> > become
>> > >> hard to test as we have dependencies on implementation everywhere.
>> > >>
>> > >> From an end user perspective the difference is only replacing
>> > >>
>> > >> X x = new X();
>> > >>
>> > >> with
>> > >>
>> > >> X x = XFactory.newInstance();
>> > >>
>> > >> But it makes it a lot easier to properly describe the functionality
>> > being
>> > >> returned. For implementation reasons, the concrete class may have
>> public
>> > >> methods that are not intended to be exposed to the application
>> > programmer
>> > >> but only to other cooperating classes. To avoid confusion, and to
>> make
>> > it
>> > >> clear to us the delta between the "shared" API and the API extensions
>> > that
>> > >> we are supporting for the pure Java implementation the interfaces seem
>> > like
>> > >> the right way to go for me.
>> > >>
>> > >> The idea is that those who need the extension features will still
>> have a
>> > >> very clear way to get the implementation that provides these, without
>> > ever
>> > >> having to cast.
>> > >>
>> > >> -- Rob
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
>> > >>
>> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
>> rob.j.godfrey@gmail.com
>> > >> > >wrote:
>> > >> >
>> > >> > > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com>
>> > >> wrote:
>> > >> > > >
>> > >> > > > As part of the Proton JNI work, I would like to remove all calls
>> > to
>> > >> > > > proton-j implementation constructors from "client code". I
>> intend
>> > >> that
>> > >> > > > factories will be used instead [1], thereby abstracting away
>> > whether
>> > >> > the
>> > >> > > > implementation is pure Java or proton-c-via-JNI.
>> > >> > > >
>> > >> > > > I'd like to check that folks are happy with me making this
>> change,
>> > >> and
>> > >> > to
>> > >> > > > mention a couple of problems I've had.
>> > >> > > >
>> > >> > > > In this context, "client code" is anything outside the current
>> > >> > > > sub-component, where our sub-components are Engine, Codec,
>> Driver,
>> > >> > > Message
>> > >> > > > and Messenger, plus each of the contrib modules, and of course
>> > third
>> > >> > > party
>> > >> > > > code.
>> > >> > > >
>> > >> > > > To enforce this abstraction, I am planning to make the
>> > constructors
>> > >> of
>> > >> > > the
>> > >> > > > affected classes package-private where possible. I believe
>> that,
>> > >> > > although
>> > >> > > > third party projects might already be calling these
>> constructors,
>> > it
>> > >> is
>> > >> > > > acceptable for us to change its public API in this manner while
>> > >> Proton
>> > >> > is
>> > >> > > > such a young project.
>> > >> > > >
>> > >> > >
>> > >> > > +1 to all of the above
>> > >> > >
>> > >> > > >
>> > >> > > > Please shout if you disagree with any of the above.
>> > >> > > >
>> > >> > > > Now, onto my problem. I started off with the
>> > >> > > org.apache.qpid.proton.engine.
>> > >> > > > impl package, and found that
>> > o.a.q.p.hawtdispatch.impl.AmqpTransport
>> > >> > > calls
>> > >> > > > various methods on ConnectionImpl and TransportImpl, so simply
>> > using
>> > >> a
>> > >> > > > Connection and Transport will not work. I don't know what to do
>> > >> about
>> > >> > > > this, and would welcome people's opinions.
>> > >> > > >
>> > >> > >
>> > >> > > So, the simplest change would be to change the factories to use
>> > >> > > covariant return types
>> > >> > >
>> > >> > > e.g. EngingFactoryImpl becomes:
>> > >> > >
>> > >> > > @Override
>> > >> > > public ConnectionImpl createConnection()
>> > >> > > {
>> > >> > > return new ConnectionImpl();
>> > >> > > }
>> > >> > >
>> > >> > > @Override
>> > >> > > public TransportImpl createTransport()
>> > >> > > {
>> > >> > > return new TransportImpl();
>> > >> > > }
>> > >> > >
>> > >> > >
>> > >> > > ... etc
>> > >> > >
>> > >> > > Code that requires the extended functionality offered by the pure
>> > Java
>> > >> > > implementation can thus instantiate the desired Factory directly.
>> > >> > >
>> > >> >
>> > >> > What's the point of going through the factory in this scenario
>> rather
>> > >> than
>> > >> > directly instantiating the classes as Hiram suggests? Is there some
>> > class
>> > >> > of thing the factory would/could do that the constructor
>> > can't/shouldn't?
>> > >> >
>> > >> > A second refinement might be to actually separate out the interface
>> > >> > > and implementation within the pure Java implementation so that we
>> > have
>> > >> > > a well defined "extended" Java API. This interface could then be
>> > the
>> > >> > > return type of the factory.
>> > >> > >
>> > >> >
>> > >> > Maybe I'm misunderstanding, but what's the point of using an
>> interface
>> > >> here
>> > >> > if you're still locked into the pure Java impl? Are you expecting to
>> > swap
>> > >> > out that impl under some circumstances?
>> > >> >
>> > >> > --Rafael
>> > >> >
>> > >>
>> > >
>> > >
>> > >
>> > > --
>> > >
>> > > **
>> > >
>> > > *Hiram Chirino*
>> > >
>> > > *Engineering | Red Hat, Inc.*
>> > >
>> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
>> redhat.com
>> > *
>> > >
>> > > *skype: hiramchirino | twitter: @hiramchirino<
>> > http://twitter.com/hiramchirino>
>> > > *
>> > >
>> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
>> >
>>
Re: Reducing the visibility of proton-j constructors
Posted by Rafael Schloming <rh...@alum.mit.edu>.
FWIW, I don't think Hiram's usage fundamentally needs to depend on the pure
Java impl. I may be out of date on this one, but I believe his access of
the implementation was done for expediency. I know at least in a number of
cases we discussed how the C implementation could accommodate his
requirements, and we plan do to so at some point. Perhaps our efforts here
would be better spent in building out the interface so that he doesn't need
direct access to the implementation.
--Rafael
On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <ph...@philharveyonline.com>wrote:
> This has implications for API versioning,
>
> I think developers of third party code can assume that the API of public
> methods, including constructors, will remain stable across minor revisions
> of a library. If a constructor is public then we, the Proton developers,
> have an obligation to respect this assumption (or at least to add "for
> internal use only" to its Javadoc).
>
> On reflection, I'm coming round to the idea that keeping these constructors
> public will place unreasonable restrictions on our ability to evolve the
> implementation whilst keeping the API stable.
>
> Phil
>
>
> On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com> wrote:
>
> > What's your need for the direct constructor?
> >
> >
> > -- Rob
> >
> > On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com> wrote:
> > > Let me clarify, I have no problem with adding Factories and using them
> > > everywhere possible. Just don't take my access away from direct
> > > constructors.
> > >
> > >
> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <rob.j.godfrey@gmail.com
> > >wrote:
> > >
> > >> In general dependency on implementation is bad, dependency on
> interface
> > is
> > >> better. One of the problems we've had with Qpid historically is it
> > become
> > >> hard to test as we have dependencies on implementation everywhere.
> > >>
> > >> From an end user perspective the difference is only replacing
> > >>
> > >> X x = new X();
> > >>
> > >> with
> > >>
> > >> X x = XFactory.newInstance();
> > >>
> > >> But it makes it a lot easier to properly describe the functionality
> > being
> > >> returned. For implementation reasons, the concrete class may have
> public
> > >> methods that are not intended to be exposed to the application
> > programmer
> > >> but only to other cooperating classes. To avoid confusion, and to
> make
> > it
> > >> clear to us the delta between the "shared" API and the API extensions
> > that
> > >> we are supporting for the pure Java implementation the interfaces seem
> > like
> > >> the right way to go for me.
> > >>
> > >> The idea is that those who need the extension features will still
> have a
> > >> very clear way to get the implementation that provides these, without
> > ever
> > >> having to cast.
> > >>
> > >> -- Rob
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
> > >>
> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
> rob.j.godfrey@gmail.com
> > >> > >wrote:
> > >> >
> > >> > > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com>
> > >> wrote:
> > >> > > >
> > >> > > > As part of the Proton JNI work, I would like to remove all calls
> > to
> > >> > > > proton-j implementation constructors from "client code". I
> intend
> > >> that
> > >> > > > factories will be used instead [1], thereby abstracting away
> > whether
> > >> > the
> > >> > > > implementation is pure Java or proton-c-via-JNI.
> > >> > > >
> > >> > > > I'd like to check that folks are happy with me making this
> change,
> > >> and
> > >> > to
> > >> > > > mention a couple of problems I've had.
> > >> > > >
> > >> > > > In this context, "client code" is anything outside the current
> > >> > > > sub-component, where our sub-components are Engine, Codec,
> Driver,
> > >> > > Message
> > >> > > > and Messenger, plus each of the contrib modules, and of course
> > third
> > >> > > party
> > >> > > > code.
> > >> > > >
> > >> > > > To enforce this abstraction, I am planning to make the
> > constructors
> > >> of
> > >> > > the
> > >> > > > affected classes package-private where possible. I believe
> that,
> > >> > > although
> > >> > > > third party projects might already be calling these
> constructors,
> > it
> > >> is
> > >> > > > acceptable for us to change its public API in this manner while
> > >> Proton
> > >> > is
> > >> > > > such a young project.
> > >> > > >
> > >> > >
> > >> > > +1 to all of the above
> > >> > >
> > >> > > >
> > >> > > > Please shout if you disagree with any of the above.
> > >> > > >
> > >> > > > Now, onto my problem. I started off with the
> > >> > > org.apache.qpid.proton.engine.
> > >> > > > impl package, and found that
> > o.a.q.p.hawtdispatch.impl.AmqpTransport
> > >> > > calls
> > >> > > > various methods on ConnectionImpl and TransportImpl, so simply
> > using
> > >> a
> > >> > > > Connection and Transport will not work. I don't know what to do
> > >> about
> > >> > > > this, and would welcome people's opinions.
> > >> > > >
> > >> > >
> > >> > > So, the simplest change would be to change the factories to use
> > >> > > covariant return types
> > >> > >
> > >> > > e.g. EngingFactoryImpl becomes:
> > >> > >
> > >> > > @Override
> > >> > > public ConnectionImpl createConnection()
> > >> > > {
> > >> > > return new ConnectionImpl();
> > >> > > }
> > >> > >
> > >> > > @Override
> > >> > > public TransportImpl createTransport()
> > >> > > {
> > >> > > return new TransportImpl();
> > >> > > }
> > >> > >
> > >> > >
> > >> > > ... etc
> > >> > >
> > >> > > Code that requires the extended functionality offered by the pure
> > Java
> > >> > > implementation can thus instantiate the desired Factory directly.
> > >> > >
> > >> >
> > >> > What's the point of going through the factory in this scenario
> rather
> > >> than
> > >> > directly instantiating the classes as Hiram suggests? Is there some
> > class
> > >> > of thing the factory would/could do that the constructor
> > can't/shouldn't?
> > >> >
> > >> > A second refinement might be to actually separate out the interface
> > >> > > and implementation within the pure Java implementation so that we
> > have
> > >> > > a well defined "extended" Java API. This interface could then be
> > the
> > >> > > return type of the factory.
> > >> > >
> > >> >
> > >> > Maybe I'm misunderstanding, but what's the point of using an
> interface
> > >> here
> > >> > if you're still locked into the pure Java impl? Are you expecting to
> > swap
> > >> > out that impl under some circumstances?
> > >> >
> > >> > --Rafael
> > >> >
> > >>
> > >
> > >
> > >
> > > --
> > >
> > > **
> > >
> > > *Hiram Chirino*
> > >
> > > *Engineering | Red Hat, Inc.*
> > >
> > > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com |
> redhat.com
> > *
> > >
> > > *skype: hiramchirino | twitter: @hiramchirino<
> > http://twitter.com/hiramchirino>
> > > *
> > >
> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
> >
>
Re: Reducing the visibility of proton-j constructors
Posted by Phil Harvey <ph...@philharveyonline.com>.
This has implications for API versioning,
I think developers of third party code can assume that the API of public
methods, including constructors, will remain stable across minor revisions
of a library. If a constructor is public then we, the Proton developers,
have an obligation to respect this assumption (or at least to add "for
internal use only" to its Javadoc).
On reflection, I'm coming round to the idea that keeping these constructors
public will place unreasonable restrictions on our ability to evolve the
implementation whilst keeping the API stable.
Phil
On 25 January 2013 14:51, Rob Godfrey <ro...@gmail.com> wrote:
> What's your need for the direct constructor?
>
>
> -- Rob
>
> On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com> wrote:
> > Let me clarify, I have no problem with adding Factories and using them
> > everywhere possible. Just don't take my access away from direct
> > constructors.
> >
> >
> > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <rob.j.godfrey@gmail.com
> >wrote:
> >
> >> In general dependency on implementation is bad, dependency on interface
> is
> >> better. One of the problems we've had with Qpid historically is it
> become
> >> hard to test as we have dependencies on implementation everywhere.
> >>
> >> From an end user perspective the difference is only replacing
> >>
> >> X x = new X();
> >>
> >> with
> >>
> >> X x = XFactory.newInstance();
> >>
> >> But it makes it a lot easier to properly describe the functionality
> being
> >> returned. For implementation reasons, the concrete class may have public
> >> methods that are not intended to be exposed to the application
> programmer
> >> but only to other cooperating classes. To avoid confusion, and to make
> it
> >> clear to us the delta between the "shared" API and the API extensions
> that
> >> we are supporting for the pure Java implementation the interfaces seem
> like
> >> the right way to go for me.
> >>
> >> The idea is that those who need the extension features will still have a
> >> very clear way to get the implementation that provides these, without
> ever
> >> having to cast.
> >>
> >> -- Rob
> >>
> >>
> >>
> >>
> >>
> >> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
> >>
> >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <rob.j.godfrey@gmail.com
> >> > >wrote:
> >> >
> >> > > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com>
> >> wrote:
> >> > > >
> >> > > > As part of the Proton JNI work, I would like to remove all calls
> to
> >> > > > proton-j implementation constructors from "client code". I intend
> >> that
> >> > > > factories will be used instead [1], thereby abstracting away
> whether
> >> > the
> >> > > > implementation is pure Java or proton-c-via-JNI.
> >> > > >
> >> > > > I'd like to check that folks are happy with me making this change,
> >> and
> >> > to
> >> > > > mention a couple of problems I've had.
> >> > > >
> >> > > > In this context, "client code" is anything outside the current
> >> > > > sub-component, where our sub-components are Engine, Codec, Driver,
> >> > > Message
> >> > > > and Messenger, plus each of the contrib modules, and of course
> third
> >> > > party
> >> > > > code.
> >> > > >
> >> > > > To enforce this abstraction, I am planning to make the
> constructors
> >> of
> >> > > the
> >> > > > affected classes package-private where possible. I believe that,
> >> > > although
> >> > > > third party projects might already be calling these constructors,
> it
> >> is
> >> > > > acceptable for us to change its public API in this manner while
> >> Proton
> >> > is
> >> > > > such a young project.
> >> > > >
> >> > >
> >> > > +1 to all of the above
> >> > >
> >> > > >
> >> > > > Please shout if you disagree with any of the above.
> >> > > >
> >> > > > Now, onto my problem. I started off with the
> >> > > org.apache.qpid.proton.engine.
> >> > > > impl package, and found that
> o.a.q.p.hawtdispatch.impl.AmqpTransport
> >> > > calls
> >> > > > various methods on ConnectionImpl and TransportImpl, so simply
> using
> >> a
> >> > > > Connection and Transport will not work. I don't know what to do
> >> about
> >> > > > this, and would welcome people's opinions.
> >> > > >
> >> > >
> >> > > So, the simplest change would be to change the factories to use
> >> > > covariant return types
> >> > >
> >> > > e.g. EngingFactoryImpl becomes:
> >> > >
> >> > > @Override
> >> > > public ConnectionImpl createConnection()
> >> > > {
> >> > > return new ConnectionImpl();
> >> > > }
> >> > >
> >> > > @Override
> >> > > public TransportImpl createTransport()
> >> > > {
> >> > > return new TransportImpl();
> >> > > }
> >> > >
> >> > >
> >> > > ... etc
> >> > >
> >> > > Code that requires the extended functionality offered by the pure
> Java
> >> > > implementation can thus instantiate the desired Factory directly.
> >> > >
> >> >
> >> > What's the point of going through the factory in this scenario rather
> >> than
> >> > directly instantiating the classes as Hiram suggests? Is there some
> class
> >> > of thing the factory would/could do that the constructor
> can't/shouldn't?
> >> >
> >> > A second refinement might be to actually separate out the interface
> >> > > and implementation within the pure Java implementation so that we
> have
> >> > > a well defined "extended" Java API. This interface could then be
> the
> >> > > return type of the factory.
> >> > >
> >> >
> >> > Maybe I'm misunderstanding, but what's the point of using an interface
> >> here
> >> > if you're still locked into the pure Java impl? Are you expecting to
> swap
> >> > out that impl under some circumstances?
> >> >
> >> > --Rafael
> >> >
> >>
> >
> >
> >
> > --
> >
> > **
> >
> > *Hiram Chirino*
> >
> > *Engineering | Red Hat, Inc.*
> >
> > *hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com
> *
> >
> > *skype: hiramchirino | twitter: @hiramchirino<
> http://twitter.com/hiramchirino>
> > *
> >
> > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
>
Re: Reducing the visibility of proton-j constructors
Posted by Rob Godfrey <ro...@gmail.com>.
What's your need for the direct constructor?
-- Rob
On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com> wrote:
> Let me clarify, I have no problem with adding Factories and using them
> everywhere possible. Just don't take my access away from direct
> constructors.
>
>
> On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <ro...@gmail.com>wrote:
>
>> In general dependency on implementation is bad, dependency on interface is
>> better. One of the problems we've had with Qpid historically is it become
>> hard to test as we have dependencies on implementation everywhere.
>>
>> From an end user perspective the difference is only replacing
>>
>> X x = new X();
>>
>> with
>>
>> X x = XFactory.newInstance();
>>
>> But it makes it a lot easier to properly describe the functionality being
>> returned. For implementation reasons, the concrete class may have public
>> methods that are not intended to be exposed to the application programmer
>> but only to other cooperating classes. To avoid confusion, and to make it
>> clear to us the delta between the "shared" API and the API extensions that
>> we are supporting for the pure Java implementation the interfaces seem like
>> the right way to go for me.
>>
>> The idea is that those who need the extension features will still have a
>> very clear way to get the implementation that provides these, without ever
>> having to cast.
>>
>> -- Rob
>>
>>
>>
>>
>>
>> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
>>
>> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <rob.j.godfrey@gmail.com
>> > >wrote:
>> >
>> > > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com>
>> wrote:
>> > > >
>> > > > As part of the Proton JNI work, I would like to remove all calls to
>> > > > proton-j implementation constructors from "client code". I intend
>> that
>> > > > factories will be used instead [1], thereby abstracting away whether
>> > the
>> > > > implementation is pure Java or proton-c-via-JNI.
>> > > >
>> > > > I'd like to check that folks are happy with me making this change,
>> and
>> > to
>> > > > mention a couple of problems I've had.
>> > > >
>> > > > In this context, "client code" is anything outside the current
>> > > > sub-component, where our sub-components are Engine, Codec, Driver,
>> > > Message
>> > > > and Messenger, plus each of the contrib modules, and of course third
>> > > party
>> > > > code.
>> > > >
>> > > > To enforce this abstraction, I am planning to make the constructors
>> of
>> > > the
>> > > > affected classes package-private where possible. I believe that,
>> > > although
>> > > > third party projects might already be calling these constructors, it
>> is
>> > > > acceptable for us to change its public API in this manner while
>> Proton
>> > is
>> > > > such a young project.
>> > > >
>> > >
>> > > +1 to all of the above
>> > >
>> > > >
>> > > > Please shout if you disagree with any of the above.
>> > > >
>> > > > Now, onto my problem. I started off with the
>> > > org.apache.qpid.proton.engine.
>> > > > impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
>> > > calls
>> > > > various methods on ConnectionImpl and TransportImpl, so simply using
>> a
>> > > > Connection and Transport will not work. I don't know what to do
>> about
>> > > > this, and would welcome people's opinions.
>> > > >
>> > >
>> > > So, the simplest change would be to change the factories to use
>> > > covariant return types
>> > >
>> > > e.g. EngingFactoryImpl becomes:
>> > >
>> > > @Override
>> > > public ConnectionImpl createConnection()
>> > > {
>> > > return new ConnectionImpl();
>> > > }
>> > >
>> > > @Override
>> > > public TransportImpl createTransport()
>> > > {
>> > > return new TransportImpl();
>> > > }
>> > >
>> > >
>> > > ... etc
>> > >
>> > > Code that requires the extended functionality offered by the pure Java
>> > > implementation can thus instantiate the desired Factory directly.
>> > >
>> >
>> > What's the point of going through the factory in this scenario rather
>> than
>> > directly instantiating the classes as Hiram suggests? Is there some class
>> > of thing the factory would/could do that the constructor can't/shouldn't?
>> >
>> > A second refinement might be to actually separate out the interface
>> > > and implementation within the pure Java implementation so that we have
>> > > a well defined "extended" Java API. This interface could then be the
>> > > return type of the factory.
>> > >
>> >
>> > Maybe I'm misunderstanding, but what's the point of using an interface
>> here
>> > if you're still locked into the pure Java impl? Are you expecting to swap
>> > out that impl under some circumstances?
>> >
>> > --Rafael
>> >
>>
>
>
>
> --
>
> **
>
> *Hiram Chirino*
>
> *Engineering | Red Hat, Inc.*
>
> *hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com*
>
> *skype: hiramchirino | twitter: @hiramchirino<http://twitter.com/hiramchirino>
> *
>
> *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
Re: Reducing the visibility of proton-j constructors
Posted by Hiram Chirino <hi...@hiramchirino.com>.
Let me clarify, I have no problem with adding Factories and using them
everywhere possible. Just don't take my access away from direct
constructors.
On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <ro...@gmail.com>wrote:
> In general dependency on implementation is bad, dependency on interface is
> better. One of the problems we've had with Qpid historically is it become
> hard to test as we have dependencies on implementation everywhere.
>
> From an end user perspective the difference is only replacing
>
> X x = new X();
>
> with
>
> X x = XFactory.newInstance();
>
> But it makes it a lot easier to properly describe the functionality being
> returned. For implementation reasons, the concrete class may have public
> methods that are not intended to be exposed to the application programmer
> but only to other cooperating classes. To avoid confusion, and to make it
> clear to us the delta between the "shared" API and the API extensions that
> we are supporting for the pure Java implementation the interfaces seem like
> the right way to go for me.
>
> The idea is that those who need the extension features will still have a
> very clear way to get the implementation that provides these, without ever
> having to cast.
>
> -- Rob
>
>
>
>
>
> On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
>
> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <rob.j.godfrey@gmail.com
> > >wrote:
> >
> > > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com>
> wrote:
> > > >
> > > > As part of the Proton JNI work, I would like to remove all calls to
> > > > proton-j implementation constructors from "client code". I intend
> that
> > > > factories will be used instead [1], thereby abstracting away whether
> > the
> > > > implementation is pure Java or proton-c-via-JNI.
> > > >
> > > > I'd like to check that folks are happy with me making this change,
> and
> > to
> > > > mention a couple of problems I've had.
> > > >
> > > > In this context, "client code" is anything outside the current
> > > > sub-component, where our sub-components are Engine, Codec, Driver,
> > > Message
> > > > and Messenger, plus each of the contrib modules, and of course third
> > > party
> > > > code.
> > > >
> > > > To enforce this abstraction, I am planning to make the constructors
> of
> > > the
> > > > affected classes package-private where possible. I believe that,
> > > although
> > > > third party projects might already be calling these constructors, it
> is
> > > > acceptable for us to change its public API in this manner while
> Proton
> > is
> > > > such a young project.
> > > >
> > >
> > > +1 to all of the above
> > >
> > > >
> > > > Please shout if you disagree with any of the above.
> > > >
> > > > Now, onto my problem. I started off with the
> > > org.apache.qpid.proton.engine.
> > > > impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
> > > calls
> > > > various methods on ConnectionImpl and TransportImpl, so simply using
> a
> > > > Connection and Transport will not work. I don't know what to do
> about
> > > > this, and would welcome people's opinions.
> > > >
> > >
> > > So, the simplest change would be to change the factories to use
> > > covariant return types
> > >
> > > e.g. EngingFactoryImpl becomes:
> > >
> > > @Override
> > > public ConnectionImpl createConnection()
> > > {
> > > return new ConnectionImpl();
> > > }
> > >
> > > @Override
> > > public TransportImpl createTransport()
> > > {
> > > return new TransportImpl();
> > > }
> > >
> > >
> > > ... etc
> > >
> > > Code that requires the extended functionality offered by the pure Java
> > > implementation can thus instantiate the desired Factory directly.
> > >
> >
> > What's the point of going through the factory in this scenario rather
> than
> > directly instantiating the classes as Hiram suggests? Is there some class
> > of thing the factory would/could do that the constructor can't/shouldn't?
> >
> > A second refinement might be to actually separate out the interface
> > > and implementation within the pure Java implementation so that we have
> > > a well defined "extended" Java API. This interface could then be the
> > > return type of the factory.
> > >
> >
> > Maybe I'm misunderstanding, but what's the point of using an interface
> here
> > if you're still locked into the pure Java impl? Are you expecting to swap
> > out that impl under some circumstances?
> >
> > --Rafael
> >
>
--
**
*Hiram Chirino*
*Engineering | Red Hat, Inc.*
*hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com*
*skype: hiramchirino | twitter: @hiramchirino<http://twitter.com/hiramchirino>
*
*blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
Re: Reducing the visibility of proton-j constructors
Posted by Rob Godfrey <ro...@gmail.com>.
In general dependency on implementation is bad, dependency on interface is
better. One of the problems we've had with Qpid historically is it become
hard to test as we have dependencies on implementation everywhere.
>From an end user perspective the difference is only replacing
X x = new X();
with
X x = XFactory.newInstance();
But it makes it a lot easier to properly describe the functionality being
returned. For implementation reasons, the concrete class may have public
methods that are not intended to be exposed to the application programmer
but only to other cooperating classes. To avoid confusion, and to make it
clear to us the delta between the "shared" API and the API extensions that
we are supporting for the pure Java implementation the interfaces seem like
the right way to go for me.
The idea is that those who need the extension features will still have a
very clear way to get the implementation that provides these, without ever
having to cast.
-- Rob
On 24 January 2013 20:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
> On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <rob.j.godfrey@gmail.com
> >wrote:
>
> > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com> wrote:
> > >
> > > As part of the Proton JNI work, I would like to remove all calls to
> > > proton-j implementation constructors from "client code". I intend that
> > > factories will be used instead [1], thereby abstracting away whether
> the
> > > implementation is pure Java or proton-c-via-JNI.
> > >
> > > I'd like to check that folks are happy with me making this change, and
> to
> > > mention a couple of problems I've had.
> > >
> > > In this context, "client code" is anything outside the current
> > > sub-component, where our sub-components are Engine, Codec, Driver,
> > Message
> > > and Messenger, plus each of the contrib modules, and of course third
> > party
> > > code.
> > >
> > > To enforce this abstraction, I am planning to make the constructors of
> > the
> > > affected classes package-private where possible. I believe that,
> > although
> > > third party projects might already be calling these constructors, it is
> > > acceptable for us to change its public API in this manner while Proton
> is
> > > such a young project.
> > >
> >
> > +1 to all of the above
> >
> > >
> > > Please shout if you disagree with any of the above.
> > >
> > > Now, onto my problem. I started off with the
> > org.apache.qpid.proton.engine.
> > > impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
> > calls
> > > various methods on ConnectionImpl and TransportImpl, so simply using a
> > > Connection and Transport will not work. I don't know what to do about
> > > this, and would welcome people's opinions.
> > >
> >
> > So, the simplest change would be to change the factories to use
> > covariant return types
> >
> > e.g. EngingFactoryImpl becomes:
> >
> > @Override
> > public ConnectionImpl createConnection()
> > {
> > return new ConnectionImpl();
> > }
> >
> > @Override
> > public TransportImpl createTransport()
> > {
> > return new TransportImpl();
> > }
> >
> >
> > ... etc
> >
> > Code that requires the extended functionality offered by the pure Java
> > implementation can thus instantiate the desired Factory directly.
> >
>
> What's the point of going through the factory in this scenario rather than
> directly instantiating the classes as Hiram suggests? Is there some class
> of thing the factory would/could do that the constructor can't/shouldn't?
>
> A second refinement might be to actually separate out the interface
> > and implementation within the pure Java implementation so that we have
> > a well defined "extended" Java API. This interface could then be the
> > return type of the factory.
> >
>
> Maybe I'm misunderstanding, but what's the point of using an interface here
> if you're still locked into the pure Java impl? Are you expecting to swap
> out that impl under some circumstances?
>
> --Rafael
>
Re: Reducing the visibility of proton-j constructors
Posted by Phil Harvey <ph...@philharveyonline.com>.
When I asked the original question I had been assuming that the contrib
modules were intended to be using the proton-api interfaces, but had to
resort to concrete types for tactical reasons pending a more complete API.
If that assumption were true, then using factory interfaces rather than
constructors would clearly be necessary.
But if this assumption is false then I personally have no problem with
concrete classes being instantiated and used directly. However, at the
risk of putting words into Rob's mouth, I guess he may be viewing the use
of concrete classes across module boundaries to be A Bad Thing that tends
to lead to leaky abstractions. Whether we apply that rule on Proton is an
interesting question, and is one of those areas where I care more about us
being consistent than about being "right".
Phil
On 24 January 2013 19:03, Rafael Schloming <rh...@alum.mit.edu> wrote:
> On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <rob.j.godfrey@gmail.com
> >wrote:
>
> > On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com> wrote:
> > >
> > > As part of the Proton JNI work, I would like to remove all calls to
> > > proton-j implementation constructors from "client code". I intend that
> > > factories will be used instead [1], thereby abstracting away whether
> the
> > > implementation is pure Java or proton-c-via-JNI.
> > >
> > > I'd like to check that folks are happy with me making this change, and
> to
> > > mention a couple of problems I've had.
> > >
> > > In this context, "client code" is anything outside the current
> > > sub-component, where our sub-components are Engine, Codec, Driver,
> > Message
> > > and Messenger, plus each of the contrib modules, and of course third
> > party
> > > code.
> > >
> > > To enforce this abstraction, I am planning to make the constructors of
> > the
> > > affected classes package-private where possible. I believe that,
> > although
> > > third party projects might already be calling these constructors, it is
> > > acceptable for us to change its public API in this manner while Proton
> is
> > > such a young project.
> > >
> >
> > +1 to all of the above
> >
> > >
> > > Please shout if you disagree with any of the above.
> > >
> > > Now, onto my problem. I started off with the
> > org.apache.qpid.proton.engine.
> > > impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
> > calls
> > > various methods on ConnectionImpl and TransportImpl, so simply using a
> > > Connection and Transport will not work. I don't know what to do about
> > > this, and would welcome people's opinions.
> > >
> >
> > So, the simplest change would be to change the factories to use
> > covariant return types
> >
> > e.g. EngingFactoryImpl becomes:
> >
> > @Override
> > public ConnectionImpl createConnection()
> > {
> > return new ConnectionImpl();
> > }
> >
> > @Override
> > public TransportImpl createTransport()
> > {
> > return new TransportImpl();
> > }
> >
> >
> > ... etc
> >
> > Code that requires the extended functionality offered by the pure Java
> > implementation can thus instantiate the desired Factory directly.
> >
>
> What's the point of going through the factory in this scenario rather than
> directly instantiating the classes as Hiram suggests? Is there some class
> of thing the factory would/could do that the constructor can't/shouldn't?
>
> A second refinement might be to actually separate out the interface
> > and implementation within the pure Java implementation so that we have
> > a well defined "extended" Java API. This interface could then be the
> > return type of the factory.
> >
>
> Maybe I'm misunderstanding, but what's the point of using an interface here
> if you're still locked into the pure Java impl? Are you expecting to swap
> out that impl under some circumstances?
>
> --Rafael
>
Re: Reducing the visibility of proton-j constructors
Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <ro...@gmail.com>wrote:
> On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com> wrote:
> >
> > As part of the Proton JNI work, I would like to remove all calls to
> > proton-j implementation constructors from "client code". I intend that
> > factories will be used instead [1], thereby abstracting away whether the
> > implementation is pure Java or proton-c-via-JNI.
> >
> > I'd like to check that folks are happy with me making this change, and to
> > mention a couple of problems I've had.
> >
> > In this context, "client code" is anything outside the current
> > sub-component, where our sub-components are Engine, Codec, Driver,
> Message
> > and Messenger, plus each of the contrib modules, and of course third
> party
> > code.
> >
> > To enforce this abstraction, I am planning to make the constructors of
> the
> > affected classes package-private where possible. I believe that,
> although
> > third party projects might already be calling these constructors, it is
> > acceptable for us to change its public API in this manner while Proton is
> > such a young project.
> >
>
> +1 to all of the above
>
> >
> > Please shout if you disagree with any of the above.
> >
> > Now, onto my problem. I started off with the
> org.apache.qpid.proton.engine.
> > impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport
> calls
> > various methods on ConnectionImpl and TransportImpl, so simply using a
> > Connection and Transport will not work. I don't know what to do about
> > this, and would welcome people's opinions.
> >
>
> So, the simplest change would be to change the factories to use
> covariant return types
>
> e.g. EngingFactoryImpl becomes:
>
> @Override
> public ConnectionImpl createConnection()
> {
> return new ConnectionImpl();
> }
>
> @Override
> public TransportImpl createTransport()
> {
> return new TransportImpl();
> }
>
>
> ... etc
>
> Code that requires the extended functionality offered by the pure Java
> implementation can thus instantiate the desired Factory directly.
>
What's the point of going through the factory in this scenario rather than
directly instantiating the classes as Hiram suggests? Is there some class
of thing the factory would/could do that the constructor can't/shouldn't?
A second refinement might be to actually separate out the interface
> and implementation within the pure Java implementation so that we have
> a well defined "extended" Java API. This interface could then be the
> return type of the factory.
>
Maybe I'm misunderstanding, but what's the point of using an interface here
if you're still locked into the pure Java impl? Are you expecting to swap
out that impl under some circumstances?
--Rafael
Re: Reducing the visibility of proton-j constructors
Posted by Rob Godfrey <ro...@gmail.com>.
On 23 January 2013 17:36, Phil Harvey <ph...@philharveyonline.com> wrote:
>
> As part of the Proton JNI work, I would like to remove all calls to
> proton-j implementation constructors from "client code". I intend that
> factories will be used instead [1], thereby abstracting away whether the
> implementation is pure Java or proton-c-via-JNI.
>
> I'd like to check that folks are happy with me making this change, and to
> mention a couple of problems I've had.
>
> In this context, "client code" is anything outside the current
> sub-component, where our sub-components are Engine, Codec, Driver, Message
> and Messenger, plus each of the contrib modules, and of course third party
> code.
>
> To enforce this abstraction, I am planning to make the constructors of the
> affected classes package-private where possible. I believe that, although
> third party projects might already be calling these constructors, it is
> acceptable for us to change its public API in this manner while Proton is
> such a young project.
>
+1 to all of the above
>
> Please shout if you disagree with any of the above.
>
> Now, onto my problem. I started off with the org.apache.qpid.proton.engine.
> impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport calls
> various methods on ConnectionImpl and TransportImpl, so simply using a
> Connection and Transport will not work. I don't know what to do about
> this, and would welcome people's opinions.
>
So, the simplest change would be to change the factories to use
covariant return types
e.g. EngingFactoryImpl becomes:
@Override
public ConnectionImpl createConnection()
{
return new ConnectionImpl();
}
@Override
public TransportImpl createTransport()
{
return new TransportImpl();
}
... etc
Code that requires the extended functionality offered by the pure Java
implementation can thus instantiate the desired Factory directly.
A second refinement might be to actually separate out the interface
and implementation within the pure Java implementation so that we have
a well defined "extended" Java API. This interface could then be the
return type of the factory.
-- Rob
>
>
> Thanks
> Phil
>
> [1] for example, these work-in-progress classes:
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/ProtonFactoryLoader.java
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/engine/EngineFactory.java
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton/src/main/java/org/apache/qpid/proton/engine/impl/EngineFactoryImpl.java
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-c/bindings/java/jni/src/main/java/org/apache/qpid/proton/engine/jni/JNIEngineFactory.java
Re: Reducing the visibility of proton-j constructors
Posted by Hiram Chirino <hi...@hiramchirino.com>.
You not actually going to prohibit folks for using the old constructors are
you?
I'd say adding factories is a good thing, and you should encourage folks to
use the factories instead of the constructors, but please don't stop folks
from using the constructors directly.
On Wed, Jan 23, 2013 at 11:36 AM, Phil Harvey <ph...@philharveyonline.com>wrote:
> As part of the Proton JNI work, I would like to remove all calls to
> proton-j implementation constructors from "client code". I intend that
> factories will be used instead [1], thereby abstracting away whether the
> implementation is pure Java or proton-c-via-JNI.
>
> I'd like to check that folks are happy with me making this change, and to
> mention a couple of problems I've had.
>
> In this context, "client code" is anything outside the current
> sub-component, where our sub-components are Engine, Codec, Driver, Message
> and Messenger, plus each of the contrib modules, and of course third party
> code.
>
> To enforce this abstraction, I am planning to make the constructors of the
> affected classes package-private where possible. I believe that, although
> third party projects might already be calling these constructors, it is
> acceptable for us to change its public API in this manner while Proton is
> such a young project.
>
> Please shout if you disagree with any of the above.
>
> Now, onto my problem. I started off with the
> org.apache.qpid.proton.engine.
> impl package, and found that o.a.q.p.hawtdispatch.impl.AmqpTransport calls
> various methods on ConnectionImpl and TransportImpl, so simply using a
> Connection and Transport will not work. I don't know what to do about
> this, and would welcome people's opinions.
>
>
> Thanks
> Phil
>
> [1] for example, these work-in-progress classes:
>
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/ProtonFactoryLoader.java
>
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton-api/src/main/java/org/apache/qpid/proton/engine/EngineFactory.java
>
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/proton/src/main/java/org/apache/qpid/proton/engine/impl/EngineFactoryImpl.java
>
> https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-c/bindings/java/jni/src/main/java/org/apache/qpid/proton/engine/jni/JNIEngineFactory.java
>
--
**
*Hiram Chirino*
*Engineering | Red Hat, Inc.*
*hchirino@redhat.com <hc...@redhat.com> | fusesource.com | redhat.com*
*skype: hiramchirino | twitter: @hiramchirino<http://twitter.com/hiramchirino>
*
*blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*