directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel L├ęcharny <elecha...@apache.org>
Subject Re: One more thing about the Control hierarchy
Date Wed, 02 Feb 2011 18:13:40 GMT
On 2/2/11 6:40 PM, Alex Karasulu wrote:
> On Wed, Feb 2, 2011 at 4:10 PM, Emmanuel Lecharny<elecharny@gmail.com>  wrote:
>> I forgot to mention that the BasicControl is inherited by 6 controls. This
>> should also be changed, having all the controls inheriting from an
>> AbstractControl class.
> Just to recap our discussion from IRC. I find value in using the
> abstract class if you need it to make final some methods you don't
> want overridden by subtypes.
Note :
The control OID and visibility fields are common to every controls.

We decided to hide the value, and we don't expose the getValue() 
setValue() and hasValue() in the Control interface.

> You mentioned though something subtle. That you would like there to be
> a distinction between two kinds of Controls produced by the codec
> service:
>
>     (1) Controls, that are not recognized by the codec and hence their
> values are treated as opaque binary data. This situation arises when
> the codec does not have a Control factory registered for that
> Control's OID. In this situation, the codec cannot decode or encode
> this Control's binary value. It just handles it by default this way to
> enable pass-thru situations where the Control's value is passed up to
> application logic for handling or handed off as-is to some other
> (internal or external) [sub]system.
>
>    (2) Registered Control factories, will supply Control extending
> interfaces exposing the Control value's internal structure with
> property accessors. Just as an example consider the Subentry control:
>
> public interface ISubentries extends Control
> {
>       String OID = "..."
>
>       void setVisibility( boolean visibility );
>
>       boolean isVisibile();
> }
The OID does not have to be exposed this way. The Subentries 
implementation can also have a constructor  like :

public class SubentriesImpl extends AbstractControl implements Subentries
{
     private String OID = "...";

     public SunbentriesImpl()
     {
         super( OID );
     }
}

Note that I extend AbstractControl, but we could also decide to extend 
BasicControl.

But that's not the point.

Let me comment further later in this mail.
> AFAIU, you want to differentiate between controls in case #1 above
> whose values are treated as opaque binary data, verses the base class
> from which all Control subtype implementations extend.
>
> If I understand correctly, then the abstractness is not the point. Let
> me suggest this alternative.

Well, my vision is that we have a couple of Interface/Implementation for 
every specific control :

ManageDsaITImpl implements ManageDsaIT
etc...

The only class we don't have this is the BasicControl class which is not 
a final class and does not have an interface except the Control 
interface. In fact, it's also used as a parent class for some of the 
controls, but not for all of them (most certainly an error) :

public class ManageDsaITImpl extends BasicControl implements ManageDsaIT
public class BasicControl implements Control
public interface ManageDsaIT extends Control

and for a control with a Value part (well, once encoded, not in the real 
control, as the value is expressed through some additional fields in the 
specific control):
public class EntryChangeImpl extends BasicControl implements EntryChange
public class BasicControl implements Control
public interface EntryChange extends Control

What bother me here is that someone who wants to create a new control 
(with or without additional fields) must instanciate a BasicControl 
directly, and  we won't be able to make a difference between such  a 
control and another specific control except by checking its OID.

Making the BasicControl an Abstract class, and creating a ControlImpl 
class for 'opaque' controls make it easier. Doing a (control instanceof 
ControlImpl) will filter out all the specific controls, while (control 
instanceof BasicControl ) won't make a difference between all the 
specific controls and opaque ones.

>     (1) For case #1 above, create a final (non-extendable) Control
> implementation class called OpaqueControl which also contains the
> additional getValue()/setValue()/hasValue() methods. We will use this
> control only for Controls we cannot handle the value of, as the
> default condition in the codec factory. This control can be in the
> model under pakcage oadl.message.controls.

To me, this OpaqueControl will be the ControlImpl. No need to call it 
Opaque, which carries some weird semantic, IMO.

But yes, we need such a class (whatever its name is)
>     (2) For case #2 above, create a concrete Control implementation
> class called BaseControl. This control should be part of the codec SPI
> instead of in the model. It should not even be exposed by the codec
> API IMO.
BaseControl in the codec ??? I'd rather have an Abstract control class, 
which won't be instanciated. Such a class is not part of the Codec 
package, I think.

>
> APPROACH BENEFITS
> ------------------------------------
>
>     o Users intuit the OpaqueControl is one whose value cannot be handled.
Agreed.
>     o The final OpaqueControl class prevents users from accidental extension.
Well, we may allow a user to extend this class. Not sure...
>     o Code that cannot access the codec, yet has access to the encoded
> value for all practical purposes see that control as opaque, if they
> need to such code can use this class to encapsulate the value to
> interface with code requiring subtypes of the model's Control
> interface, the codec can convert it if need be deep below. This is the
> only exception to the visibility of the value property and this way it
> is OK.
Agreed.
>      o The BaseControl is hidden and can only be used by codec
> extenders as part of the SPI
Again, I don't see why we should hide the BaseControl in the codec. We 
already have decorators for that. And if this OpaqueControl has a 
getValue() setValue() methods, the getValue() method will return a 
byte[], so it does not need to have a specific encoder/decoder. Although 
we need to create an OpaqueControlDecorator to manage the envelop encoding.

>      o When the codec cannot handle a control it creates the OpaqueControl type.
Agreed.
> On another note we might need a final OpaqueControlDecorator which
> does not need to keep a value member as it's own member but can access
> the value via OpaqueControl.getValue().

Absolutely.


-- 
Regards,
Cordialement,
Emmanuel L├ęcharny
www.iktek.com


Mime
View raw message