| Enrique Rodriguez 2006-05-16, 1:11 pm |
| ersiner-1oDqGaOF3Lkdnm+yROfE0A@public.gmane.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
|