On Thu, 2005-05-26 at 04:41 +0200, [EMAIL PROTECTED] wrote: > Well, the change broke no unit tests, so I'm committing it. All converter > classes in the converters package have been made non-final.
I'm not entirely convinced this patch is a good idea. When a class is final, there are no arguments about what methods/members should be protected vs private. I bet now these classes are non-final that we get bugzilla entries asking for methods/members to be made protected or public :-) When users can subclass a class, you must consider two things: * that removing protected methods/members is an incompatible change * that changing the internal logic to call protected methods in a different way (including not calling them at all) is an incompatible change, even though not a compile or link problem, as code that overrides the protected method will no longer work as expected. As things currently stand, there are no protected members/methods in the converter classes [1] so this isn't an issue. Well, BooleanArrayConverter actually does have a protected member, though it used to be a final class :-). So we should fix this to make it private, yes? And of course final classes are potentially faster than non-final ones. Not really a major issue in beanutils given that it uses so much reflection already but it's worth having if it doesn't do any harm. So the question is what kind of gain there is from this change. How many converters actually have enough logic in them that it is worth subclassing them rather than implementing a custom converter from scratch? If BooleanConverter is the only converter with significant logic in id, then maybe we would be better off enhancing BooleanConverter by adding a constructor to take custom array of strings rather than allowing it to be subclassed? Or maybe users could use the Decorator pattern rather than subclassing to extend its functionality? Regards, Simon --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]