You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by Rafael Schloming <rh...@alum.mit.edu> on 2014/05/01 21:39:48 UTC

Re: simplified factory pattern for proton-j

Just a heads up, I've committed this to trunk. Please let me know if it
causes any problems.

--Rafael


On Tue, Apr 29, 2014 at 2:57 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> Hi Everyone,
>
> I've put together a patch that makes the proton-j factory usage a bit
> simpler and more consistent. You can review it here if you like:
>
>   - https://reviews.apache.org/r/20854/
>
> The main point of the patch is to make all the factories consistently
> follow this pattern:
>
>   package.Interface iface = package.Interface.Factory.create(...);
>
> I like this because it is simple and easy to remember and doesn't require
> importing the impl packages directly.
>
> The patch preserves the convenience constructors, e.g. Proton.connection()
> and so forth, but it does remove the old factory APIs. I think this is a
> reasonable thing to do because the old factories were cumbersome enough to
> use that I don't think anyone actually bothered (including our own
> exmaples).
>
> In any case, please shout if this patch will be troublesome for you. If I
> don't hear anything I'll go ahead and commit it later this week.
>
> Thanks,
>
> --Rafael
>
>

Re: simplified factory pattern for proton-j

Posted by Clebert Suconic <cs...@redhat.com>.

-- Clebert Suconic typing on the iPhone. 

> On Jun 24, 2014, at 11:44 PM, Clebert Suconic <cs...@redhat.com> wrote:
> 
> I just bumped into one issue (actually two issues) after this:
> 
> 
> I - First HAWT-JMS is not working with trunk because the API is changed here. I could change the API but now I have an issue that we can't commit the change since there is no commit for this yet.
> 

I meant. There is no version for this yet. 
> II - Using this Factory on the interface makes it very tight to use any other factory. I mean.. why use a factory at all if the only factory available is the one described on the Interface? you could just use new MessageImpl. 
>    The advantage of the factory would be to plug other message implementations. For instance I just came up with a message implementation on my JMS parser that doesn't parse the body (since my server doesn't need to do it). I'm not using any of the factories here because of this. 
>     I don't want to make a big deal about this.. but I guess I would prefer the old factories back.
> 
> 
> 
> 
>> On May 1, 2014, at 3:39 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:
>> 
>> Just a heads up, I've committed this to trunk. Please let me know if it
>> causes any problems.
>> 
>> --Rafael
>> 
>> 
>>> On Tue, Apr 29, 2014 at 2:57 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:
>>> 
>>> Hi Everyone,
>>> 
>>> I've put together a patch that makes the proton-j factory usage a bit
>>> simpler and more consistent. You can review it here if you like:
>>> 
>>> - https://reviews.apache.org/r/20854/
>>> 
>>> The main point of the patch is to make all the factories consistently
>>> follow this pattern:
>>> 
>>> package.Interface iface = package.Interface.Factory.create(...);
>>> 
>>> I like this because it is simple and easy to remember and doesn't require
>>> importing the impl packages directly.
>>> 
>>> The patch preserves the convenience constructors, e.g. Proton.connection()
>>> and so forth, but it does remove the old factory APIs. I think this is a
>>> reasonable thing to do because the old factories were cumbersome enough to
>>> use that I don't think anyone actually bothered (including our own
>>> exmaples).
>>> 
>>> In any case, please shout if this patch will be troublesome for you. If I
>>> don't hear anything I'll go ahead and commit it later this week.
>>> 
>>> Thanks,
>>> 
>>> --Rafael
> 

Re: simplified factory pattern for proton-j

Posted by Clebert Suconic <cs...@redhat.com>.
I just bumped into one issue (actually two issues) after this:


I - First HAWT-JMS is not working with trunk because the API is changed here. I could change the API but now I have an issue that we can't commit the change since there is no commit for this yet.

II - Using this Factory on the interface makes it very tight to use any other factory. I mean.. why use a factory at all if the only factory available is the one described on the Interface? you could just use new MessageImpl. 
    The advantage of the factory would be to plug other message implementations. For instance I just came up with a message implementation on my JMS parser that doesn't parse the body (since my server doesn't need to do it). I'm not using any of the factories here because of this. 
     I don't want to make a big deal about this.. but I guess I would prefer the old factories back.




On May 1, 2014, at 3:39 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:

> Just a heads up, I've committed this to trunk. Please let me know if it
> causes any problems.
> 
> --Rafael
> 
> 
> On Tue, Apr 29, 2014 at 2:57 PM, Rafael Schloming <rh...@alum.mit.edu> wrote:
> 
>> Hi Everyone,
>> 
>> I've put together a patch that makes the proton-j factory usage a bit
>> simpler and more consistent. You can review it here if you like:
>> 
>>  - https://reviews.apache.org/r/20854/
>> 
>> The main point of the patch is to make all the factories consistently
>> follow this pattern:
>> 
>>  package.Interface iface = package.Interface.Factory.create(...);
>> 
>> I like this because it is simple and easy to remember and doesn't require
>> importing the impl packages directly.
>> 
>> The patch preserves the convenience constructors, e.g. Proton.connection()
>> and so forth, but it does remove the old factory APIs. I think this is a
>> reasonable thing to do because the old factories were cumbersome enough to
>> use that I don't think anyone actually bothered (including our own
>> exmaples).
>> 
>> In any case, please shout if this patch will be troublesome for you. If I
>> don't hear anything I'll go ahead and commit it later this week.
>> 
>> Thanks,
>> 
>> --Rafael
>> 
>>