commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benedikt Ritter (JIRA)" <>
Subject [jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
Date Fri, 02 May 2014 10:48:15 GMT


Benedikt Ritter commented on SANDBOX-462:

bq. when one AccessibleObject is resolved directly, why isn't it put to the cache? [1]

I don't know :-) The code was extracted from BeanUtils 1 and it may contain bugs that show
up when starting to refactor. You have all the tests in place. If putting the AO into the
cache doesn't break the code, feel free to change it.

when no best match could be obtained (I assume this is possible because of bestMatch!= null),
null would be put in cache. This will do not big damage directly, but wouldn't this in theory
slow down the whole routine significantly because of a very large key set which has to be
searched on every run (see line 90 in [3]). [2]

I don't know if this slows things down ;-) It often turns out that performance issues are
hiding where you don't expect them, so I tend not to talk about performance without measurements.
If you want to go into performance tuning, have a look at SANDBOX-432 first.


> Refactoring of AccessibleObjectsRegistry
> ----------------------------------------
>                 Key: SANDBOX-462
>                 URL:
>             Project: Commons Sandbox
>          Issue Type: Improvement
>          Components: BeanUtils2
>            Reporter: Andre Diermann
>            Priority: Minor
>         Attachments: Commons-BeanUtils2-462#1.patch
> Summary:
> The AccessibleObjectsRegistry class provides two get methods, while one is a convenient
method for the other.
> Both methods take one conditional parameter, boolean exact, and the actual get method
is very long, which makes it somehow complex to understand.
> Suggestion:
> What could be improved IMHO:
> - Instead of using conditional methods, like get(boolean doSomethingSpecialIfTrue, ...),
it is more convenient to provide dedicated methods like getSomething() and getAnotherThing().
> - In this regard the difference between an exact or, let's call it, matching descriptor
should be expressed through inheritance rather than object allocation (= expressing it by
a field boolean exact).
> - The very long get method should be refined
> - Another very minor issue is the naming of the paramTypes field within the inner AccessibleObjectDescriptor
class, which I would suggest to rename to parameterTypes to fit the naming of the other occurrences.

This message was sent by Atlassian JIRA

View raw message