commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simone Tripodi <simonetrip...@apache.org>
Subject Re: [SANDBOX][BeanUtils2] Thoughts about AccessibleObjectsRegistry
Date Sat, 28 Jan 2012 07:26:51 GMT
Hi Benedikt,
thanks for investing your spare time on contributing on BeanUtils2!
Please read my inline comments:

> Complex methods: I know, that BeanUtils2 is just an experiment, and that the
> code was written to get it working fast for this purpose. But some methods
> are really hard to read. Take for example resolve() on MethodsRegistry.
> There are at least two things happening: 1. try to find a direct match, 2.
> if no direct match can be found, start a some how more complex search. I
> think this could be separated into several methods. If you agree, I can
> create an issue and write a patch.

what is the advantage of splitting that method? the algorithm looks clear:

  input: methodName, parameterTypes
  output: c

  get the direct method
      return if found
  else
      for each method with given name
          if parameterTypes.length != method argument type length
              skip
          for each method argument type
              check if input argument type matches with current argument type
          if method matches
              bestMatchMethod <- calculateCost( method ) // this is
where the complex has been already split
      return bestMatchMethod

can you describe please how you would intend splitting the method?

>
> Magic numbers: I really don't understand, how object transformation costs
> get calculated in MethodsRegistry.getObjectTransformationCosts. What does
> 0.25f mean? And why do we add 1.5f? Are this values you came up with from
> the development of BeanUtils1? If so, this should be documented in the code.
>

exactly, this has been merely copied from BeanUtils1 and it is not
clear to me as well - we should go reading the mail archive and/or
commit history to understand where they come from and comment,
extracting magic number as constants etc...

>
> Terminology: I really don't like the name of the class. My opinion is, that
> the term "object" should never be overloaded. The problem is, that
> AccessibleObjects refer to methods and constructors that are accessible from
> the caller (e.g. public for a caller outside the containing package of the
> bean class). Now, if you are new to BeanUtils2, your first thought might be,
> that those registries are holding objects, that are accessible themselves.
> What I'm trying to say is, that maybe the name should be changed to
> something more convenient, for example "AccessibleOperationsRegistry". Since
> the general definition of a class is, that it defines the data and possible
> operations on that data for a set of similar objects, I think that name
> would be very appropriate (AccessibleObjectDescriptor, etc should be renamed
> likewise).
>

-1 it is not a matter of taste, it comes from the nature of the
objects that the registry stores: both Method and Constructor
implement the AccessibleObject[1] interface, so a generic
implementation that stores AccessibleObject extensions, reused for
storing Method and Constructor, how else shall be called?
I don't like the therm `AccessibleObject` as well but I bet I would
require ages to get it changed in JVM spec... :)

looking forward to read more from you, have a nice weekend, alles gute!
-Simo

[1] http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/reflect/AccessibleObject.html

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message