[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[
https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13987578#comment-13987578
]
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.
{quote}
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]
{quote}
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.
HTH,
Benedikt
> Refactoring of AccessibleObjectsRegistry
>
>
> Key: SANDBOX-462
> URL: https://issues.apache.org/jira/browse/SANDBOX-462
> 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
(v6.2#6252)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13987571#comment-13987571 ] Andre Diermann commented on SANDBOX-462: Sure, waiting for your response on my questions two comments before :-) > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > 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 (v6.2#6252)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13987566#comment-13987566 ] Benedikt Ritter commented on SANDBOX-462: - Any news here? Do you want to provide a second patch? ;-) > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > 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 (v6.2#6252)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13924869#comment-13924869 ] Andre Diermann commented on SANDBOX-462: Very nice! I already used it for issue #463 :) Many thanks! > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > 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 (v6.2#6252)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[
https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13924827#comment-13924827
]
Benedikt Ritter commented on SANDBOX-462:
-
Hi André,
{quote}
any hints how to check the rules and format the code already on my site? Do you
have a code style config for your IDE? Do you run checkstyle locally? Currently
I don't even know what I messed up exactly :-/
{quote}
I've changed the site build of BeanUtils2 to use commons parent pom.xml. You
can now call mvn site again. It will generate the component website into the
target directory. Just open index.html and have a look at the check style
report. There a currently a lot of violations already that we will need to fix
and contributions should at best not introduce any new violations :-)
HTH,
Benedikt
> Refactoring of AccessibleObjectsRegistry
>
>
> Key: SANDBOX-462
> URL: https://issues.apache.org/jira/browse/SANDBOX-462
> 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
(v6.2#6252)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13919666#comment-13919666 ] Andre Diermann commented on SANDBOX-462: I have two questions regarding the current routine of get(): - when one AccessibleObject is resolved directly, why isn't it put to the cache? [1] - 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] [1] see line 90 - 104 in [3] [2] see line 152 - 156 in [3] [3] http://svn.apache.org/viewvc/commons/sandbox/beanutils2/trunk/src/main/java/org/apache/commons/beanutils2/AccessibleObjectsRegistry.java?view=markup > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > 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 (v6.2#6252)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[
https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13914666#comment-13914666
]
Benedikt Ritter commented on SANDBOX-462:
-
Hello André
usually I've used mvn site and then looked at the check style report on the
generated site. Unfortunately the site build is currently broken. I have to
either release a new version of the sandbox parent pom find a different
solution for this problem.
To see what I've changed you can call
{code}
svn diff -r 1570311:1570312
src/main/java/org/apache/commons/beanutils2/AccessibleObjectsRegistry.java
{code}
Was just about whitespaces, as you can see.
Regards,
Benedikt
> Refactoring of AccessibleObjectsRegistry
>
>
> Key: SANDBOX-462
> URL: https://issues.apache.org/jira/browse/SANDBOX-462
> 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
(v6.1.5#6160)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13908570#comment-13908570 ] Andre Diermann commented on SANDBOX-462: Hi Benedikt, any hints how to check the rules and format the code already on my site? Do you have a code style config for your IDE? Do you run checkstyle locally? Currently I don't even know what I messed up exactly :-/ So sorry for the additional effort any many thanks for applying the patch. :-) I will now continue with the "bigger" changes. See you, André > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > 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 (v6.1.5#6160)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13907349#comment-13907349 ] Benedikt Ritter commented on SANDBOX-462: - Hello André, this patch looks much better. I've applied it in r1570311. They patch does not completely obey to the checkstyle rules in the project. I've correct this in r1570312. I don't like this maven code style myself, so I'm currently thinking about proposing two change the checkstyle rules. But for now, we should stick with it. Thanks! Benedikt > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > 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 (v6.1.5#6160)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13906030#comment-13906030 ] Andre Diermann commented on SANDBOX-462: Hi Benedikt, thank you for the explanation. I have chosen option b) :-) I added one new attachment (Commons-BeanUtils2-462#1.patch) containing one really minor change. I will provide a patch with the "bigger" changes in the next couple of days. BR, André > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > Project: Commons Sandbox > Issue Type: Improvement > Components: BeanUtils2 >Reporter: Andre Diermann >Priority: Minor > Attachments: Commons-BeanUtils2-462#1.patch, > Commons-BeanUtils2-462.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 (v6.1.5#6160)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[
https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13905893#comment-13905893
]
Benedikt Ritter commented on SANDBOX-462:
-
Hello André,
SVN can only create a patch against the BASE revision of or working copy (that
is the revision that was HEAD in the repo, when you called svn up the last
time). So if you add change after change, they will all end up in the patch.
To make this clearer:
* You update to HEAD, let's say that's revision 19.
* You make a change to file A.java and create a patch via {{svn diff A.java}}.
The patch will contain all changes in A.java in your working copy compared to
the base revision (which is 19).
* Now you make another change to A.java that is unrelated to the first change
and create a new patch. The patch will again be created against the base
revision. So it has to create all the changes you made before.
Just ask yourself: how could svn know about which changes have to be combined
for a patch. The fact that your changes had a temporal ordering is not
available to SVN. It's just something that you and may be the editing history
of your IDE knows.
So you either have to:
a) make your changes, create the patch and revert to your base revision, or
b) make your changes, create the patch, then wait until the patch has been
applied, update to head and then create the next patch
HTH,
Benedikt
> Refactoring of AccessibleObjectsRegistry
>
>
> Key: SANDBOX-462
> URL: https://issues.apache.org/jira/browse/SANDBOX-462
> Project: Commons Sandbox
> Issue Type: Improvement
> Components: BeanUtils2
>Reporter: Andre Diermann
>Priority: Minor
> Attachments: Commons-BeanUtils2-462.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
(v6.1.5#6160)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13905796#comment-13905796 ] Andre Diermann commented on SANDBOX-462: Hello Benedict, you suggested to split my changes into multiple patch files, which is a good advice and makes totally sense. :-) Somehow I haven't found an option how to do this. The patch is always created depending on the server version. For instance: - I changed one minor thing -> created and commented a patch for that (no problems so far) - I changed another part within the same file, here: AccessibleObjectsRegistry -> try to create a patch -> the patch also contains the changes of the first patch - ... - So in the end the last patch file will be the same as already provided (of course except the unrelated changes due to the formatting ;-)), but still with too many changes. So did I understand you right? Regards, André > Refactoring of AccessibleObjectsRegistry > > > Key: SANDBOX-462 > URL: https://issues.apache.org/jira/browse/SANDBOX-462 > Project: Commons Sandbox > Issue Type: Improvement > Components: BeanUtils2 >Reporter: Andre Diermann >Priority: Minor > Attachments: Commons-BeanUtils2-462.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 (v6.1.5#6160)
[jira] [Commented] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[
https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13903410#comment-13903410
]
Benedikt Ritter commented on SANDBOX-462:
-
Hello André,
first of all: thanks for this contribution! I've reviewed it but I think we
have to tidy it up a bit before we can get it into trunk :-) Here are my
findings:
>From the description I can see that you have at least four changes in mind. It
>is better to keep the change sets of patches as small as possible. That makes
>it a lot easier to review patches. So I'd suggest that you create one patch
>for every bullet point.
Your patch contains a lot of unrelated changes, mostly due to auto formatting
of your IDE. See for example this lines in your patch file:
{code}
-abstract class AccessibleObjectsRegistry
-{
+abstract class AccessibleObjectsRegistry
{
{code}
This change just places the curly brace at the end of the line. Please remove
this unrelated changes. BeanUtils uses the maven coding conventions. A fact
that I'm not completely happy with because no IDE has this as a standard
configuration. But for now it is as it is and we should stick with it until we
have come to another decision on the ML.
I can see in your patch file that you've used your IDE to generate it. This is
perfectly fine, but I personally prefer the command line, since it gives you
finer control over what happens. Maybe you should give it a try?
As far as I can see you haven't yet filed an
[ICLA|http://www.apache.org/licenses/]. Please do so as soon as you find the
time. If you're planning to contribute on a regular basis, this is an mandatory
requirement.
If you have any problems with getting the patch right, don't hesitate to ask
here or on the ML.
Regards,
Benedikt
> Refactoring of AccessibleObjectsRegistry
>
>
> Key: SANDBOX-462
> URL: https://issues.apache.org/jira/browse/SANDBOX-462
> Project: Commons Sandbox
> Issue Type: Improvement
> Components: BeanUtils2
>Reporter: Andre Diermann
>Priority: Minor
> Attachments: Commons-BeanUtils2-462.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
(v6.1.5#6160)
