On Wed, Mar 30, 2011 at 4:50 PM, Pierre-Arnaud Marcelot <pa@marcelot.net> wrote:
Hi Alex,

On 30 mars 2011, at 14:24, Alex Karasulu wrote:



On Wed, Mar 30, 2011 at 12:20 PM, Pierre-Arnaud Marcelot <pa@marcelot.net> wrote:
Hi Emmanuel,

+1

SearchScope could be in 'org.apache.directory.shared.ldap.model.message'.
LdapURL could in 'org.apache.directory.shared.ldap.model.url'.

Shouldn't we also rename 'LdapURL' as 'LdapUrl' to match other class names (like Dn, Rdn, Oid, etc.)?

Regards,
Pierre-Arnaud


My same thoughts. LdapUrl should go into message pkg too though - why have an extra package with just one or two classes?

Readability and longevity I guess.
We might need to add more classes related to LdapUrl later, like an URL factory or more.
Having this extra package ensures that we won't be tempted to move it afterwards, breaking compatibility in client applications.


This does make sense, the longevity argument. As you say who knows what else will creep up around LdapUrl. OK I am on the same page now especially after your comments below showing there no longer is a dependency from the 'message' package.
 
At first, I was hesitating between either 'org.apache.directory.shared.ldap.model' or 'org.apache.directory.shared.ldap.model.url'.

Are you picking 'message' because LdapUrl is somehow related to referrals?

Yep that was my motivation for selecting 'message'. 
 
I've just verified and none of the classes under the 'message' package uses LdapUrl (not even 'Referral').
Other than that, I don't really see the link between the classes under this package and the LdapUrl class...


Oh that's a surprise to me - guess we may be calculating it then populating with Strings. I thought there would naturally be a dependency there, my bad.

Best,
Alex