You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Emmanuel Lecharny <el...@gmail.com> on 2011/03/30 02:31:25 UTC
Some few classes that are probably stored at the right place...
Hi guys,
while I was reviewing the Filter classes, I found a couple of classes in
the filter packages and I don't think they should be stored under this
package :
- SearchScope : an Enum used in SearchRequests
- LdapURL : used mostly in referrals
Anyone has an idea about the best location for those 2 classes ? in
'message' ?
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: Some few classes that are probably stored at the right place...
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 3/30/11 11:20 AM, Pierre-Arnaud Marcelot 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.)?
Yes, absolutely.
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: Some few classes that are probably stored at the right place...
Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
On 30 mars 2011, at 19:14, Alex Karasulu wrote:
>
>
> 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.
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
Re: Some few classes that are probably stored at the right place...
Posted by Alex Karasulu <ak...@apache.org>.
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
Re: Some few classes that are probably stored at the right place...
Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
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.
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?
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...
Regards,
Pierre-Arnaud
>
> Regards,
> Alex
Re: Some few classes that are probably stored at the right place...
Posted by Alex Karasulu <ak...@apache.org>.
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?
Regards,
Alex
Re: Some few classes that are probably stored at the right place...
Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
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
On 30 mars 2011, at 02:31, Emmanuel Lecharny wrote:
> Hi guys,
>
> while I was reviewing the Filter classes, I found a couple of classes in the filter packages and I don't think they should be stored under this package :
> - SearchScope : an Enum used in SearchRequests
> - LdapURL : used mostly in referrals
>
> Anyone has an idea about the best location for those 2 classes ? in 'message' ?
>
> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>