Thanks for your comments Nail.
I think that I've come around to see your point after sleeping on it.
What do you think about this:
Context.java - would be defined as so:
public interface Context<K extends Object, V extends Object> extends Map<K, V>
Then ContextBase.java would be defined like so:
public class ContextBase extends ConcurrentHashMap<String, Object>
implements Context<String, Object> {
Or we could change String to CharSequence (but I will leave that for
another discussion).
@Simo - would this negatively affect the retrieve methods that you wrote?
Thanks,
-Elijah
On Thu, Sep 8, 2011 at 5:39 PM, Niall Pemberton
<niall.pember...@gmail.com> wrote:
> On Fri, Sep 9, 2011 at 12:25 AM, Elijah Zupancic <eli...@zupancic.name> wrote:
>> Hi Niall,
>>
>> The source of the misunderstanding regarding the usage of chain may be
>> my fault. Thank you very much for piping up and letting us know some
>> of the history regarding the chain project.
>>
>> I was under the assumption that all keys of a Context were String
>> because in ContextBase in the initialization method we have:
>>
>> // Retrieve the set of property descriptors for this Context class
>> try {
>> pd = Introspector.getBeanInfo
>> (getClass()).getPropertyDescriptors();
>> } catch (IntrospectionException e) {
>> pd = new PropertyDescriptor[0]; // Should never happen
>> }
>>
>> // Initialize the underlying Map contents
>> for (int i = 0; i < pd.length; i++) {
>> String name = pd[i].getName();
>>
>> // Add descriptor (ignoring getClass() and isEmpty())
>> if (!("class".equals(name) || "empty".equals(name))) {
>> if (descriptors == null) {
>> descriptors = new HashMap<String,
>> PropertyDescriptor>((pd.length - 2));
>> }
>> descriptors.put(name, pd[i]);
>> super.put(name, singleton);
>> }
>> }
>>
>> When you look at the method signature on FeatureDesriptor for
>> getName() for the following call:
>>
>> String name = pd[i].getName();
>>
>> you will see that the only acceptable choice is a string. Thus, if you
>> are subclassing ContextBase, you have to use Strings as keys in order
>> to make the BeanUtils glue work or you have to have a beanless
>> context.
>
> Yes that is certainly true with the ContextBase implementation and the
> use-case (Struts) that drove the development of Chain wanted exactly
> that - a typed bean that could be treated as a Map.
>
> http://svn.apache.org/repos/asf/struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/contexts/
>
> From memory (its been a while since I committed on Struts), Struts
> only ever accessed its context through the bean properties and not
> through the Map API. However Chain's contract never limited it to that
> use-case, just provided the ContextBase implementation to make it
> easy.
>
>> I'm of the opinion that standardizing on String or <? extends
>> CharSequence> as the generic key for Context will make using Context
>> far more usable. Otherwise, if you use a non-string key, you will be
>> fighting many parts of the API that assume a String key.
>
> I would agree it makes it more useable where someone wants to define
> their context as a bean with strongly typed properties. But you're
> putting a limit on the API that isn't there and I can't think of a
> single benefit that this brings. If someone chooses to use
> ContextBase, then fine they accept that limitation. I don't see how
> you believe it will be more useable - seems the opposite to me if I
> can no longer do something that I used to be able to. I also don't
> understand the comment about " fighting many parts of the API" - it
> seems to me that outside of ContextBase it has no impact.
>
> Niall
>
>> Also, what made me assume that the contract was for String-only keys
>> was the fact that the test cases only use String keys (that is unless
>> my memory is failing me).
>>
>> Thanks,
>> -Elijah
>>
>> On Thu, Sep 8, 2011 at 1:53 PM, Niall Pemberton
>> <niall.pember...@gmail.com> wrote:
>>> On Tue, Sep 6, 2011 at 9:39 AM, Simone Tripodi <simonetrip...@apache.org>
>>> wrote:
>>>> Hi Niall,
>>>> thanks for the hint!
>>>>
>>>> Anyway (DISCLAIMER: I'm putting in the original chain author's shoes,
>>>> so I couldn't say the truth) I immagine that users would be interested
>>>> on having, as a Context, not just a place where storing computed
>>>> element to be shared between chain commands, but having also the
>>>> possibility of customizations adding, for example, shared clever
>>>> methods - take a look at the concrete default
>>>> {{org.apache.commons.chain.impl.ContextBase}} implementation where
>>>> there is an index of PropertiesDescriptors.
>>>
>>> I understand what Chain does - I was the last active Chain committer.
>>> I was also around when it was developed for Struts.
>>>
>>> You miss the point I make though. Context is currently an interface
>>> that extends the Map interface - it adds nothing, zilch, nada, rien to
>>> the Map interface
>>>
>>> public interface Context extends Map {
>>> }
>>>
>>> So the only thing having "Context" does is it prevents use of any
>>> standard Map implementation. It doesn't prevent any fancy or clever
>>> implementations you want to create - but just restricts what you can
>>> pass through the Chain.
>>>
>>> Also I just looked at your changes to the Context definition and
>>> you're now adding a second restriction - that the keys to the Context
>>> have to now be a String. Thats great for people who effectively want a
>>> property name - but its a new limitation for those that don't and I
>>> don't see any benefit to that limitation.
>>>
>>> Niall
>>>
>>>
>>>> Honestly thinking, after raw your message, I'd tend to agree that
>>>> Map<String,Object> would be more than enough - just for the record,
>>>> that's what we deed indeed in the Apache Cocoon3 Pipeline APIs - but
>>>> at the same time I like the idea of having dedicated Chain API as
>>>> shown in the current implementation.
>>>>
>>>> Hard to take a decision...
>>>> Simo
>>>>
>>>> http://people.apache.org/~simonetripodi/
>>>> http://www.99soft.org/
>>>>
>>>>
>>>>
>>>> On Tue, Sep 6, 2011 at 2:19 AM, Niall Pemberton
>>>> <niall.pember...@gmail.com> wrote:
>>>>> On Mon, Sep 5, 2011 at 12:21 PM, James Carman
>>>>> <ja...@carmanconsulting.com> wrote:
>>>>>> I agree with Paul here. Extending Map (or any other class for that
>>>>>> matter) when what you really want to do is encapsulate it is silly.
>>>>>> Is the Context really meant to be used in any place where a Map can be
>>>>>> used? I would think not.
>>>>>
>>>>> I always thought the other way. I never understood why context wasn't
>>>>> just a Map, rather than a new Chain specific type extending Map.
>>>>>
>>>>> Using Map has its advantages. Firstly the contract it provides besides
>>>>> get/put are useful operations on the context (containsKey(), size(),
>>>>> entrySet() etc.etc) , secondly (if it was a "plain" Map) there are
>>>>> standard implementations & wrappers that can be used giving features
>>>>> such as concurrency, ready-only etc. and thirdly tools & libraries
>>>>> understand how to operate on a Map.
>>>>>
>>>>> Niall
>>>>>
>>>>>> On Sun, Sep 4, 2011 at 11:54 PM, Paul Benedict <pbened...@apache.org>
>>>>>> wrote:
>>>>>>> I want to get rid of it extending map. Have it define as asMap()
>>>>>>> function instead. Especially since JDK 8 is bringing in extension
>>>>>>> methods, which adds new (and default) methods to all collections, it
>>>>>>> won't look very nice. Let's make a break now.
>>>>>>>
>>>>>>> On Sun, Sep 4, 2011 at 9:20 PM, Raman Gupta <rocketra...@fastmail.fm>
>>>>>>> wrote:
>>>>>>>> On 09/04/2011 04:00 PM, James Carman wrote:
>>>>>>>>> On Sun, Sep 4, 2011 at 3:44 PM, Simone Tripodi
>>>>>>>>> <simonetrip...@apache.org> wrote:
>>>>>>>>>>
>>>>>>>>>> That is able to 'auto-cast' the retrieved object while Map#get() not.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I believe the feature is actually called "type inference", not
>>>>>>>>> "auto-cast." :)
>>>>>>>>
>>>>>>>> Thanks for the explanation... I see now that via the generic method
>>>>>>>> the compiler infers the return type from the assignment type.
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Raman
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
