commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niall Pemberton <niall.pember...@gmail.com>
Subject Re: [chain][v2] clever context
Date Fri, 09 Sep 2011 00:39:01 GMT
On Fri, Sep 9, 2011 at 12:25 AM, Elijah Zupancic <elijah@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.pemberton@gmail.com> wrote:
>> On Tue, Sep 6, 2011 at 9:39 AM, Simone Tripodi <simonetripodi@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.pemberton@gmail.com> wrote:
>>>> On Mon, Sep 5, 2011 at 12:21 PM, James Carman
>>>> <james@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 <pbenedict@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 <rocketraman@fastmail.fm>
wrote:
>>>>>>> On 09/04/2011 04:00 PM, James Carman wrote:
>>>>>>>> On Sun, Sep 4, 2011 at 3:44 PM, Simone Tripodi <simonetripodi@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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message