directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Enrique Rodriguez <enriqu...@gmail.com>
Subject Re: svn commit: r406888 - /directory/trunks/apacheds/kerberos-shared/src/main/java/org/apache/directory/server/kerberos/shared/store/operations/GetPrincipal.java
Date Tue, 16 May 2006 16:55:39 GMT
ersiner@apache.org wrote:
...
> +    
> +    /**
> +     * TODO: It's better to move this method to a common place.
> +     */
> +    private static boolean parseBoolean( String bool )
> +    {
> +        if ( bool.equals( "true" ) )
> +        {
> +            return true;
> +        }
> +
> +        return false;
> +    }
> +    
>  }

This entire method is the same as saying:

return bool.equals( "true" );

In which case, you could inline it:

modifier.setLockedOut( val.toLowerCase().equals( "true" ) );

Or, Emmanuel's suggestion:

modifier.setLockedOut( "true".equalsIgnoreCase( val ) );

Now, you have a negligible amount of code that almost added a dependency 
on shared-ldap.  Besides bloating the kerberos code by 3 X there is also 
no conceptual link between kerberos and shared-ldap code.  The problem 
here is not necessary code size, which in this day and age isn't that 
important, but rather the conceptual integrity of the artifacts we 
provide and a drastic increase in POM maintenance in product 
configurations as these sorts of deps trickle down.  I'm sorry to use 
Ersin's commit as an example, but this sort of "eliminate duplication at 
all costs" mindset occurs quite a bit on this project and it makes POM 
maintenance in downstream product assemblies a lot harder.

Moreover, the thing to recognize here is that we have configuration code 
in apacheds-core and protocol-common and helper methods like the above 
in many of the protocols and shared-ldap.  In which case, the thing to 
do is form a configuration package.  What I mean by "conceptual 
integrity" is that we could then point to a configuration artifact as 
the place for configuration library and utility code, as opposed to 
pointing to shared-ldap or apacheds-core or protocol-common, which have 
nothing to do, conceptually, with boolean text parsing or configuration.

Enrique

Mime
View raw message