On 2/2/11 6:40 PM, Alex Karasulu wrote:
On Wed, Feb 2, 2011 at 4:10 PM, Emmanuel Lecharny<elecha...@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
