Matthew Swift a écrit :
On 13/01/10 21:55, Stefan Seelmann wrote:
Matthew Swift wrote:
On 12/01/10 14:29, Emmanuel Lecharny wrote:
Even if you decide that caching is not required then that's no
reason to develop an API which prevents you from implementing one in
future. Using a normal constructor prevents the use of a cache (or
forces you to use the pimpl idiom). If you extend the API later by
adding a valueOf method then no existing applications will be able
to take advantage of the perf improvement unless they are modified
to use the new static factory method.
Matthew, you mentioned good reasons for factory methdods. I think it
is not a big deal to add a one or two factory methods.
rootDN/nullDN/emptyDN is an obvious candidate
Yeah. I think that nullDN is a nonsense. rootDN is probably technically
the correct vision, but I'm not sure that users will understand the
implication of such a name. I would rather pick emptyDN just because
it's semantically not tainted (ie, rootDN <=> roodDSE DN), and an empty
DN can be the base for operations like move, applied in the middle of
the tree, not at the root (assumong you can add RDN to a DN).
valueOf(String) is the other and perhaps valueOf(ByteString/byte[])
We had a long discussion with Ludovic while in Portland. We have made a
big mistake one year and a half ago trying to introduce something like a
Value<?> object (where ? = String or byte[]). I think it's a nonsense
too. valueOf(String) and valueOf(byte[]) clearly have my preference over
any other form. No need to use a ByteString, it huld be purely internal.
I don't like having two ways to achieve the same goal, hence my
dislike of the DN(String) constructor, especially when users of the
method will not automatically inherit performance improvements in
future releases.
But for users, DN(String) covers 99% of their usage. This is how they
created DN with JNDI, and i'm not sure they want something very
different. Now, internally, othing prevent you to write something like :
public DN(String dnStr) {
return valueOf( dnStr );
}
Also, on the subject of AVAs - we have the AVA type as an inner
class in RDN. I'm not particularly happy with this this, but less
happy with it being a standalone class since AVAs are only used in
RDNs and may introduce confusion elsewhere. For example, filters
also use attribute value assertions but these are not the same type
of object as an AVA even if they have the same name. For example,
AVAs (in RDNs) do not allow attribute options or matching rules to
be specified.
What do you think? Inner class or standalone?
It could be an inner class, but should be visible and constructable
from outside. The use case I see is to create an DN or RDN by
specifying the attribute types and values, without having to deal
with escaped characters. We need such functionality in Directory
Studio, when defining the RDN of an entry or when renaming an entry.
The user for example just types "a+b" into the value field, we
construct a new AVA("cn", "a+b") and the AVA implementation should
handle the escaping to "cn=a\+b".
There's no need for an AVA class at all from a construction point of
view. For example, you could have an RDN method RDN.addAVA(type,
value). The stronger requirement for an AVA class comes when you need
to access the AVAs in an RDN since you want to get two values at once
- the attribute type and attribute value. It is possible to avoid
having an AVA class altogether by having methods like:
int getAVACount()
AttributeType getAVAAttributeType(int index)
AttributeValue getAVAAttributeValue(int index)
void addAVA(AttributeType, AttributeValue)
One less class is a good thing IMO especially when it's potentially a
source of confusion (with filters), but I'm not a big fan of the
indexing and the inability to easily iterate over the AVAs. I'm pretty
undecided on this one.
I also have in mind serialization and deserialization : having the AVA
class halps a bt to track down the way it's done.
Now, when you face DN like cn=ACME+gn=whatever, manipulating AVAs is
convenient, as you can iterate though them instead of using indices.
