One more thing about the Control hierarchy

2011-02-02 Thread Emmanuel Lecharny
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.


--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com



Re: One more thing about the Control hierarchy

2011-02-02 Thread Alex Karasulu
On Wed, Feb 2, 2011 at 4:10 PM, Emmanuel Lecharny  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.

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();
}

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.

   (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.

   (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.


APPROACH BENEFITS


   o Users intuit the OpaqueControl is one whose value cannot be handled.
   o The final OpaqueControl class prevents users from accidental extension.
   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.
o The BaseControl is hidden and can only be used by codec
extenders as part of the SPI
o When the codec cannot handle a control it creates the OpaqueControl type.

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().

WDYT?

Best,
- Alex


Re: One more thing about the Control hierarchy

2011-02-02 Thread Emmanuel Lécharny

On 2/2/11 6:40 PM, Alex Karasulu wrote:

On Wed, Feb 2, 2011 at 4:10 PM, Emmanuel Lecharny  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 OpaqueCont