Nigel,
On 2013-01-22, at 12:29, Nigel Deakin <nigel.deakin_at_oracle.com> wrote:
> If it's backed by the JMSProducer's internal map, why do you think it needs to be unmodifiable? There's no technical
> reason why this needs to be the case. This is just a simple map (and does not need to be threadsafe).
It could be that the producer is backed by a native system, so the map would be as well. Changing the map would have to be forwarded to that native system. While quite possible, it would be extra effort over the dedicated methods to add/remove/change the properties. Maybe the native system doesn't distinguish between header fields and properties... things can get ugly for the implementers.
> I think the set returned should either be
>
> * backed by the JMSProducer's internal map, in which case it should follow the semantics of Map.keySet()
>
> * a simple copy of the keys in the JMSProducer's internal map, in which case it will follow the obvious semantics of a copy.
>
> My feeing is that to make it "backed", but not allow the application to delete properties by removing property names
> from the set, would be unnecessarily inconsistent. It should either be "fully" backed, or a simple copy.
Right, this would have to be consistent.
I think the question we should ask is: Do developers want to modify the properties map/set directly or are the methods sufficient? If the latter, make it immutable.
Having a copy wouldn't help anybody, would it? It would just cause extra load on the GC (maps are comparably huge!). This reminds me of the Swing issues with all those defensive copies of the mutable Rectangle/Point classes.
Rüdiger
> On 21/01/2013 21:40, John D. Ament wrote:
>> Nigel,
>>
>> If it's backed by the Map's entrySet then it should definitely be unmodifiable. The set is unmodifiable, if you make
>> changes to the underlying map they should be reflected in the map.
>>
>>
>> On Mon, Jan 21, 2013 at 10:29 AM, Nigel Deakin <nigel.deakin_at_oracle.com <mailto:nigel.deakin_at_oracle.com>> wrote:
>>
>> John,
>>
>>
>> On 21/01/2013 13:51, John D. Ament wrote:
>>
>> Hi Nigel,
>>
>> If there are no properties set on the JMSProducer, will this return null or the equivalent of
>> Collections.EMPTY_SET ?
>>
>>
>> It returns an empty Set. Returning null would force the application to add code to handle this case.
>>
>>
>> Since you should not modify it, can we call it an immutable set in the API docs (e.g. a wrap of
>> Collections.unmodifiableSet ?)
>>
>>
>> Rüdiger did suggest that and I didn't respond to it. Collections.unmodifiableSet returns a set backed by the
>> underlying collection. But if we're going to return a "backed" set, then I don't particularly see any reason why it
>> needs to be unmodifiable in this particular case.
>>
>> Although I did originally suggested that this Set be a copy, there's no reason in practice why it needs to be, and
>> it might be simpler to just adopt the same behaviour as Map#keySet, which returns "a Set view of the keys contained
>> in the map".
>>
>> Remember that all a JMSProducer does is to maintain a Map of property name/value pairs which will be copied to any
>> message which is sent.
>>
>> The implementation would be trivial, given that the JMSProducer would probably be maintaining these properties in a
>> Map anyway.
>>
>> The API doc would be:
>>
>> ------------------------------__------------------------------__------
>> Set<String> getPropertyNames()
>>
>> Returns a Set view of the names of all the message properties that have been set on this JMSProducer.
>>
>>
>> Note that JMS standard header fields are not considered properties and are not returned in this Set.
>>
>> The set is backed by the JMSProducer, so changes to the map are reflected in the set, and vice-versa. Its behaviour
>> matches that defined in the java.util.Map method keySet.
>>
>> Returns:
>>
>> a Set containing the names of all the message properties that have been set on this JMSProducer
>> Throws:
>> JMSRuntimeException - if the JMS provider fails to get the property names due to some internal error.
>> See Also:
>> Map.keySet()
>> ------------------------------__------------------------------__------
>>
>> Is this better?
>>
>> Nigel
>>
>>
>>
>> John
>>
>>
>>
>> On Mon, Jan 21, 2013 at 7:53 AM, Nigel Deakin <nigel.deakin_at_oracle.com <mailto:nigel.deakin_at_oracle.com>
>> <mailto:nigel.deakin_at_oracle.__com <mailto:nigel.deakin_at_oracle.com>>> wrote:
>>
>> Following comments, I have now drafted this small API change. You can view it at
>> http://jms-spec.java.net/2.0-____SNAPSHOT/apidocs/javax/jms/____JMSProducer.html#____getPropertyNames%28%29
>> <http://jms-spec.java.net/2.0-__SNAPSHOT/apidocs/javax/jms/__JMSProducer.html#__getPropertyNames%28%29>
>>
>> <http://jms-spec.java.net/2.0-__SNAPSHOT/apidocs/javax/jms/__JMSProducer.html#__getPropertyNames%28%29
>> <http://jms-spec.java.net/2.0-SNAPSHOT/apidocs/javax/jms/JMSProducer.html#getPropertyNames%28%29>>
>>
>> getPropertyNames
>>
>> Set<String> getPropertyNames()
>>
>> Returns an Set containing the names of all the message properties that have been set on this JMSProducer.
>>
>> Note that JMS standard header fields are not considered properties and are not returned in this Set.
>>
>> The returned Set is a copy. Changes to the JMSProducer will not be reflected in the Set, nor vice-versa.
>>
>> Returns:
>> a Set containing the names of property values
>> Throws:
>> JMSRuntimeException - if the JMS provider fails to get the property names due to some internal error.
>>
>> Nigel
>>
>>
>> On 11/01/2013 16:07, Nigel Deakin wrote:
>>
>> I'd like to propose we accept Philippe's proposal and change the definition of
>> JMSProducer#getPropertyNames() to
>> return
>> a Set <String> instead of an Enumeration.
>>
>> I propose that for simplicity this be a copy of the property names. It would not be backed by the
>> underlying
>> collection,
>> unlike Map#keySet. I cannot conceive of a use case where that would be needed.
>>
>> Although this is a minor change, if we are going to make it we have to make it before JMS 2.0 is
>> released, since we
>> won't be able to change it after that.
>>
>> Any comments?
>>
>> Nigel
>>
>>
>>
>> On 07/01/2013 12:40, Nigel Deakin wrote:
>>
>> Philippe - Thank you for your detailed and well-argued message. I think you make a very good case
>> for changing
>> JMSProducer#getPropertyNames
>> from
>> Enumeration getPropertyNames()
>> to
>> Set<String>
>>
>> I think that probably is a good change.
>>
>> However I think it is worth me pointing out that this method (either version) will be used rarely,
>> since all
>> it does it
>> allow an application to discover what property names were set using prior calls to
>> JMSProducer#setProperty()
>> on the same
>> JMSProducer object.
>>
>> Although JMSProducer#getPropertyNames() and the various JMSProducer#get*Property methods such as
>> JMSProducer#getLongProperty() were added for completeness and consistency I think there are few use
>> cases
>> where they
>> would be used. So are they needed at all?
>>
>> Nigel
>>
>>
>> On 04/01/2013 18:57, Philippe Marschall wrote:
>>
>>
>>
>> On 04.01.2013 11:57, Nigel Deakin wrote:
>>
>> Hi Philippe,
>>
>> On 03/01/2013 22:14, Philippe Marschall wrote:
>>
>> Hi
>>
>> Why is the return type of JMSProducer#getProperyNames() a raw (!)
>> Enumeration instead of a Set<String>?
>>
>>
>> Good question. This method (and many other methods on JMSProducer) is
>> simply a copy of the corresponding method on Message. And
>> Message#getPropertyNames returns a "raw" Enumeration.
>>
>> http://docs.oracle.com/javaee/____6/api/javax/jms/Message.__html#__getPropertyNames%28%29
>> <http://docs.oracle.com/javaee/__6/api/javax/jms/Message.html#__getPropertyNames%28%29>
>>
>> <http://docs.oracle.com/__javaee/6/api/javax/jms/__Message.html#getPropertyNames%__28%29
>> <http://docs.oracle.com/javaee/6/api/javax/jms/Message.html#getPropertyNames%28%29>>
>>
>>
>>
>> (Both methods return a collection of message property names)
>>
>> Hmm. There are two issues here:
>>
>> 1. whether it should return an Enumeration or a Set
>> 2. whether it should use a generic type, i.e. <String>
>>
>> In general I think we should avoid try to keep the methods on
>> JMSProducer and Message consistent since the user will need to use both.
>>
>> (The exception to this is that whereas Message has nine methods to set
>> properties with names like setStringProperty, setShortProperty etc,
>> JMSProducer has nine methods all called setProperty. So I have broken my
>> own "rule" in this case in order to simplify the API.)
>>
>>
>> My understanding is that JMSProducer is the "new, clutter free, boiler
>> plate free, concise" API. Enumeration itself is a pretty useless
>> interface. There are no useful methods like #contains or #size, you
>> can't put it into an enhanced-for-loop and it likely won't work with
>> Java 8 "functional methods". You always have to iterate which is
>> inefficient or convert to a Set which is boiler plate.
>>
>> It starts with simple things, iteration with a Set you can do
>>
>> for (String properytName : producer.getProperyNames()) {
>>
>> }
>>
>> with an enumeration you do
>>
>> Enumeration<?> enumeration = producer.getProperyNames();
>> while (enumeration.hasMoreElements()____) {
>>
>> String properytName = enumeration.nextElement();
>> }
>>
>> but goes on to things like #contains, with a Set you can do:
>>
>> producer.getProperyNames().____contains("JMSXProducerTXID")
>>
>>
>> but with an enumeration you write the following which is probably also
>> less efficient because you do a linear scan
>>
>> boolean contains = false;
>> Enumeration<?> enumeration = producer.getProperyNames();
>> while (enumeration.hasMoreElements()____) {
>> if ("JMSXProducerTXID".equals(____enumeration.nextElement())) {
>>
>> contains = true;
>> break;
>> }
>> }
>>
>> With Java 8 (which should ship this year) and a Set you can do:
>>
>> Set<String> jmsxProperties = producer.getProperyNames().____filter(p ->
>>
>> p.startsWith("JMSX"));
>>
>> If you don't return a Set you'll have to do
>>
>> Set<String> jmsxProperties = new HashSet<>();
>> Enumeration<?> enumeration = producer.getProperyNames();
>> while (enumeration.hasMoreElements()____) {
>>
>> String nextElement = (String) enumeration.nextElement();
>> if (nextElement.startsWith("JMSX"____)) {
>> jmsxProperties.add(____nextElement);
>>
>> }
>> }
>>
>>
>>
>> My feeling is that returning a generic type would be a fairly
>> transparent improvement.
>>
>> However, given that Message#getPropertyNames returns an Enumeration I
>> think it might be confusing if JMSProducer#getPropertyNames returns a
>> Set.
>>
>>
>> Look at your current code that uses #getProperyNames and look how much
>> simpler, easier to understand and less error prone it would be when
>> #getProperyNames returned a Set.
>>
>> Could we at least have a second method that returns a Set?
>>
>> So I suggest we change this to return an Enumeration <String>.
>>
>>
>> Cheers
>> Philippe
>>
>>
>>
- application/pkcs7-signature attachment: smime.p7s