On 30 mars 2011, at 19:14, Alex Karasulu wrote: > > > On Wed, Mar 30, 2011 at 4:50 PM, Pierre-Arnaud Marcelot <[email protected]> > 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 <[email protected]> >> 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.
It surprised me a lot too. I was really expecting it to be used there. Maybe we should fix that, or at least open a JIRA about it... > > Best, > Alex
