On 9/8/2010 10:40 AM, Adam Lally wrote:
> On Fri, Sep 3, 2010 at 3:28 PM, Marshall Schor <[email protected]> wrote:
>
>> There are different kinds of things one could do here. One would be to add
>> some
>> more notes to the manual(s) (I didn't check, the notes might actually
>> already be
>> there, but we could in that case make them stand out more :-) ) saying that
>> if
>> you use the base implementation classes as superclasses of your annotator
>> implementation, you *must* call super.initialize.
>>
> Another would be to add some support in the getContext() default method
>> which
>> checks for null, and if found, issues a longer error message stating that
>> the in
>> user's implementations of initialize which extend xyz base class, the user
>> *must* call super.initialize..., before calling getContext() or other
>> framework
>> methods that call getContext() (such as getEmptyCAS - used in CAS
>> multipliers),
>> or words to that effect.
>>
>> I agree with doing the above - better documentation and error messages are
> always good.
>
>
>
>> Another thing that could be done is to have the framework examine the
>> annotator
>> class before it calls its initialize method, to see if it is extending one
>> of
>> the base impl classes which has this field and makes use of it in the
>> getContext() method, and if it finds that to be true, the framework could
>> set
>> this field itself. This would eliminate the possible error from happening,
>> but
>> maybe it's overkill for what is a simple thing for users to do.
>>
>>
> My thinking on this is that this adds additional complication that's not
> worth it. Currently we follow a straightfoward and often-used convention:
>
> * The annotator interface defines the API between framework and the user's
> component
> * The abstract base class provides default implementations of the methods,
> as a convenience
>
> Your suggestion breaks this convention, basically by having the abstract
> base class expose additional functionality (like a setContext() method) that
> the framework knows about and calls. In my experience things are not
> normally done that way, so developers may be confused by what is happening.
> It's kind of a "back door" way of accessing the annotator.
>
> I also do not think that calling super.initialize() is that unusual of a
> requirement when overriding a method.
>
> I admit that the current annotator interface is not ideal. If I had it to
> do over again, I would have the UimaContext be a parameter to the
> reconfigure method. But that is not really feasible at this point. IMO, the
> proposed patches for this problem all seem to have more downsides (in added
> complexity) than upsides.
>
> -Adam
>