[jira] [Updated] (LANG-965) FieldUtils methods leak accessible flags
[
https://issues.apache.org/jira/browse/LANG-965?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andre Diermann updated LANG-965:
Attachment: commons-lang-965.patch
Ok, after some further analysis my understanding is as follows:
o accessibility is changed explicitly in
- {{readField(final Field field, final Object target, final boolean
forceAccess)}} and
- {{writeField(final Field field, final Object target, final Object value,
final boolean forceAccess)}}
o furthermore, accessibility may be changed implicitly in every read/write
method which needs to resolve a field by name first, when calling
- {{getField(final Class? cls, final String fieldName, final boolean
forceAccess)}} or
- {{getDeclaredField(final Class? cls, final String fieldName, final
boolean forceAccess)}}
So my proposal is to have only one place where accessibility is changed, which
will simplify restoring of accessibility a lot. Therefore I added two methods
for the internal resolution of fields, which do not modify accessibility. I
made those two methods private, so public API is not changed.
FieldUtils methods leak accessible flags
Key: LANG-965
URL: https://issues.apache.org/jira/browse/LANG-965
Project: Commons Lang
Issue Type: Bug
Components: lang.reflect.*
Affects Versions: 3.1, 3.2.1
Environment: Apache Maven 3.1.1
(0728685237757ffbf44136acec0402957f723d9a; 2013-09-17 11:22:22-0400)
Maven home: C:\Java\apache-maven-3.1.1\bin\..
Java version: 1.7.0_51, vendor: Oracle Corporation
Java home: C:\Program Files\Java\jdk1.7.0_51\jre
Default locale: en_US, platform encoding: Cp1252
OS name: windows 7, version: 6.1, arch: amd64, family: windows
Reporter: Gary Gregory
Assignee: Benedikt Ritter
Fix For: Review Patch
Attachments: commons-lang-965.patch, commons-lang-965.patch
When various FieldUtils methods are called the accessible is set to true but
never reset to false. This is side-effect should be cleaned up.
This makes a mess of the object model which represents the class meta data.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (LANG-965) FieldUtils methods leak accessible flags
[
https://issues.apache.org/jira/browse/LANG-965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14112567#comment-14112567
]
Andre Diermann commented on LANG-965:
-
Hello Benedikt,
many thanks for your remarks.
bq. a new issue for this small change is a bit overkill. Just add it to your
patch.
Done.
bq. yes, but correctness of readField() is asserted by individual unit tests
for that method, so I think it's okay to use it in the test for writeField() if
this makes the test code easier to understand.
Done. It's really more readable like this.
bq. So the idea is use getField with forceAccess = false to retrieve the field.
This will return the field, even if it is not accessible. After that you can
simply call the other writeField() and pass the forceAccess parameter into it.
That method in turn will handle restoring of the accessibility flag for you.
I changed this as well and the code looked really nice ;-) ... but
unfortunately it broke the tests, because this changed the required behavior of
the method :-(
forceAccess is a little bit misleading here, because it does not mean always
assure setAccessible(true) is called, it rather means break the scope or
more precisely also search superclasses for this field. So when trying to
read or write a field with the proposed implementation, the field could not be
found anymore if it is specified in a superclass.
After that I investigated the code of {{FieldUtils}} more and discovered that
my patch does not cover all accessibility changes :-( It seems the
accessibility is changed literally everywhere ;-) I followed a lot of paths and
finally came to the conclusion that my original approach of use the minimum
amount of code changes is not sufficient to cope with this problem.
So I will work on a more elaborated patch. Just want to inform you that this
might take a while, because the underlying issue is more complex than I
initially expected O:-)
Best regards,
André
FieldUtils methods leak accessible flags
Key: LANG-965
URL: https://issues.apache.org/jira/browse/LANG-965
Project: Commons Lang
Issue Type: Bug
Components: lang.reflect.*
Affects Versions: 3.1, 3.2.1
Environment: Apache Maven 3.1.1
(0728685237757ffbf44136acec0402957f723d9a; 2013-09-17 11:22:22-0400)
Maven home: C:\Java\apache-maven-3.1.1\bin\..
Java version: 1.7.0_51, vendor: Oracle Corporation
Java home: C:\Program Files\Java\jdk1.7.0_51\jre
Default locale: en_US, platform encoding: Cp1252
OS name: windows 7, version: 6.1, arch: amd64, family: windows
Reporter: Gary Gregory
Assignee: Benedikt Ritter
Fix For: Review Patch
Attachments: commons-lang-965.patch
When various FieldUtils methods are called the accessible is set to true but
never reset to false. This is side-effect should be cleaned up.
This makes a mess of the object model which represents the class meta data.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (LANG-965) FieldUtils methods leak accessible flags
[
https://issues.apache.org/jira/browse/LANG-965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14111332#comment-14111332
]
Andre Diermann commented on LANG-965:
-
Hi Benedikt,
bq. Maybe we should add a comment in JavaDoc, that users have to take care of
restoring accessibility when using getField()...
Yes, adding a comment in the corresponding JavaDoc is a good idea and will
raise the attention for a potential accessibility leak a bit more. Should I
change the JavaDoc accordingly as part of this issue or should this be a
dedicated issue and patch?
bq. However I think it would be better to use FieldUtils.readField() to check
if the field has been set correctly and to assert that the field is not
accessible after the call to writeField(). WDYT?
I think the author of the original test code wanted to use pure Java for the
assertion. The correct behavior of {{Field.get()}} is guaranteed by the JDK
while {{FieldUtils.readField()}} is not (since this method is tested in the
same test class and execution order of jUnit tests might be random). So from my
understanding the avoidance of a method of the class under test to test another
method within the class under test is on purpose. So my intention here was
really to use the minimum amount of changes to the original code to keep the
tests green and this one-liner was the shortest solution.
Maybe this is a smell that tests for the correct restoring of the accessibility
is missing? O:) I will add further tests to assert that accessibility is
restored correctly. This should not be tested implicitly within the
{{testWrite…()}}. Ok?
bq. 1. missing curly brackets ;-) We use curly brackets even for one line
blocks.
Sorry, you are write. I will change this.
bq. 2. I'm not sure whether {{!Modifier.isPublic(field.getModifiers()))}}
really is the same as {{Field.isAccessible() == false}}. Reading through the
JavaDoc of {{Field.setAccessible(boolean)}} sounds like it is a difference. But
I don't know. It think it would be better to get the field via {{getField(cls,
fieldName, false)}}, store the accessibility, do all neccessary operations and
then restore the original state. This way you only need to restore the
accessibility once.
The reason for the usage of {{Modifier.isPublic()}} is the used workaround for
JDK 1.3 in {{getField()}} (see line 107). The accessibility of the field is
only set to true, when forceAccess is true and the modifier is public. That’s
why I need to check both before evantually restoring accessibility to false.
But I totally agree with you, that it looks confusing. I don’t know if we can
solve this without jeopardizing the workaround.
Sorry if the patch is that confusing, but I really spent a lot of time to
understand the original code and then tried to use a minimum amount of changes
to fix issue #965. Thus, I am afraid some of your remarks address the original
code as well. :-/
See you,
André
FieldUtils methods leak accessible flags
Key: LANG-965
URL: https://issues.apache.org/jira/browse/LANG-965
Project: Commons Lang
Issue Type: Bug
Components: lang.reflect.*
Affects Versions: 3.1, 3.2.1
Environment: Apache Maven 3.1.1
(0728685237757ffbf44136acec0402957f723d9a; 2013-09-17 11:22:22-0400)
Maven home: C:\Java\apache-maven-3.1.1\bin\..
Java version: 1.7.0_51, vendor: Oracle Corporation
Java home: C:\Program Files\Java\jdk1.7.0_51\jre
Default locale: en_US, platform encoding: Cp1252
OS name: windows 7, version: 6.1, arch: amd64, family: windows
Reporter: Gary Gregory
Assignee: Benedikt Ritter
Fix For: Review Patch
Attachments: commons-lang-965.patch
When various FieldUtils methods are called the accessible is set to true but
never reset to false. This is side-effect should be cleaned up.
This makes a mess of the object model which represents the class meta data.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (LANG-965) FieldUtils methods leak accessible flags
[
https://issues.apache.org/jira/browse/LANG-965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14107998#comment-14107998
]
Andre Diermann commented on LANG-965:
-
Hello Benedikt,
sorry for inconvenience. Let me clarify:
- Restoring accessibility in {{readField(Object, String, boolean)}} and
{{writeField(Object, String, Object, boolean)}} is necessary, because
getField() gets called in these two methods. getField() may change the
accessibility of the field. Since getField() guarantees, according to its
javadoc, to return an accessible {@link Field} by name it was not an option
to restore the accessibility within this method. Therefore it needs to be
restored in the above mentioned read/write methods separately.
- The original tests failed after the accessibility leak has been fixed. In
the corresponding assert statement the field was filled with an expected value
for assertion although it might not be accessible. Before, they only succeeded
because of the leak. You can reproduce this by applying the patch for the
FieldUtil class and use the original tests. So the changes to the tests do not
test the restoring of the accessibility, they just guarantee, that the tests
work like before.
Let me know, if any uncertainty remains.
BR
André
FieldUtils methods leak accessible flags
Key: LANG-965
URL: https://issues.apache.org/jira/browse/LANG-965
Project: Commons Lang
Issue Type: Bug
Components: lang.reflect.*
Affects Versions: 3.1, 3.2.1
Environment: Apache Maven 3.1.1
(0728685237757ffbf44136acec0402957f723d9a; 2013-09-17 11:22:22-0400)
Maven home: C:\Java\apache-maven-3.1.1\bin\..
Java version: 1.7.0_51, vendor: Oracle Corporation
Java home: C:\Program Files\Java\jdk1.7.0_51\jre
Default locale: en_US, platform encoding: Cp1252
OS name: windows 7, version: 6.1, arch: amd64, family: windows
Reporter: Gary Gregory
Assignee: Benedikt Ritter
Fix For: Review Patch
Attachments: commons-lang-965.patch
When various FieldUtils methods are called the accessible is set to true but
never reset to false. This is side-effect should be cleaned up.
This makes a mess of the object model which represents the class meta data.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Updated] (LANG-965) FieldUtils methods leak accessible flags
[ https://issues.apache.org/jira/browse/LANG-965?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated LANG-965: Attachment: commons-lang-965.patch The attached patch tries so solve the mentioned accessibility leak by restoring the original accessibility of a particular field in case it has been modified by readField/writeField. FieldUtils methods leak accessible flags Key: LANG-965 URL: https://issues.apache.org/jira/browse/LANG-965 Project: Commons Lang Issue Type: Bug Components: lang.reflect.* Affects Versions: 3.1, 3.2.1 Environment: Apache Maven 3.1.1 (0728685237757ffbf44136acec0402957f723d9a; 2013-09-17 11:22:22-0400) Maven home: C:\Java\apache-maven-3.1.1\bin\.. Java version: 1.7.0_51, vendor: Oracle Corporation Java home: C:\Program Files\Java\jdk1.7.0_51\jre Default locale: en_US, platform encoding: Cp1252 OS name: windows 7, version: 6.1, arch: amd64, family: windows Reporter: Gary Gregory Assignee: Gary Gregory Fix For: Patch Needed Attachments: commons-lang-965.patch When various FieldUtils methods are called the accessible is set to true but never reset to false. This is side-effect should be cleaned up. This makes a mess of the object model which represents the class meta data. -- 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-tabpanelfocusedCommentId=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-464) Add support for interpreting property expressions
[
https://issues.apache.org/jira/browse/SANDBOX-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13951831#comment-13951831
]
Andre Diermann commented on SANDBOX-464:
Hi Benedikt,
if you put it like this we should maybe discuss if it make sense in general to
use expressions along with a fluent API which is in turn designed to express an
expression.
Of course allowing expressions in BU1 makes the use of the API a way more
convenient. But the design of BU2's API already covers that.
So by allowing both it is a mitigation of the fluent API itself and may lead to
very poor code [3]. On the other hand a fluent API call may become very long
[1]. In relation to this providing a dedicated convenient method for
expressions would IMHO add some comfort for an API user [2]. For instance:
[1]
on(addressBook).getMapped(provider).with(Google).getIndexed(contact).at(5).getMapped(address).with(home).get(street)
[2]
on(addressBook).getExpressed(provider(google).contact[5].address(home).street)
[3]
on(addressBook).get(provider(google).contact[5]).getMapped(address).with(home).get(street.yetAnotherNestedProperty)
So, what you can see is that in [1] there is a clean elegant type safe call,
which unfortunately could become quite long. [2] somehow mitigates the fluent
API but shortens the call. May be good for API users who know their beans very
good and put convenience over type safety. Allowing [3], which is what you are
thinking of in your comment above, would IMHO permit and encourage the user to
mix everything up.
In conclusion, providing a dedicated method for expressions as convenience
would make it explicit for the user what he is doing. I think that is legal.
But mixing the fluent API with expressions (which means to somehow overload
each method and making it do several things at once depending on what the user
passes as parameter) is poor IMHO. This could be the source of many runtime
errors because it is less strict and the user may not know what he is doing
(because a lot of things happens under the hood which he is not aware of).
Again, I think we should have in mind why expressions where nice to use in BU1
but the API of BU2 is already designed to represent expressions itself.
Cheers,
André
Add support for interpreting property expressions
-
Key: SANDBOX-464
URL: https://issues.apache.org/jira/browse/SANDBOX-464
Project: Commons Sandbox
Issue Type: Task
Components: BeanUtils2
Affects Versions: Nightly Builds
Reporter: Benedikt Ritter
Attachments: Commons-Sandbox-464-Get.patch
BeanUtils 1 has the functionality to interpret property paths. For example
the call:
{code:java}
BeanUtils.getProperty(adress.street, person);
{code}
would first get the address of the given person and then get the street from
the address and return it. The same is possible for list and mapped
properties.
In order to be supplement for BeanUtils1, we need to implement this in
BeanUtils2.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Commented] (SANDBOX-464) Add support for interpreting property expressions
[
https://issues.apache.org/jira/browse/SANDBOX-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13944414#comment-13944414
]
Andre Diermann commented on SANDBOX-464:
Hello Benedikt,
I see you agree that there should be two distinct methods for the two purposes.
:-) On the other hand I understand your concern and like your idea to find
another name for the method that interprets property expressions. Therefore I
extended the API with a method called getExpressed() (in regards to
getIndexed() and getMapped()). The default get() method remained untouched.
Due to the current abilities of the PropertyInterpreter I makes no sense to
scale the patch by following the above's subtask structure. Hence I rather
split the work in two parts, part 1 contains all the getter stuff, while part 2
will contain everything related to setting properties using expressions.
Please find part 1 in attached patch file Commons-Sandbox-464-Get.patch.
What do you think?
Regards,
André
Add support for interpreting property expressions
-
Key: SANDBOX-464
URL: https://issues.apache.org/jira/browse/SANDBOX-464
Project: Commons Sandbox
Issue Type: Task
Components: BeanUtils2
Affects Versions: Nightly Builds
Reporter: Benedikt Ritter
BeanUtils 1 has the functionality to interpret property paths. For example
the call:
{code:java}
BeanUtils.getProperty(adress.street, person);
{code}
would first get the address of the given person and then get the street from
the address and return it. The same is possible for list and mapped
properties.
In order to be supplement for BeanUtils1, we need to implement this in
BeanUtils2.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Updated] (SANDBOX-464) Add support for interpreting property expressions
[
https://issues.apache.org/jira/browse/SANDBOX-464?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andre Diermann updated SANDBOX-464:
---
Attachment: Commons-Sandbox-464-Get.patch
Add support for interpreting property expressions
-
Key: SANDBOX-464
URL: https://issues.apache.org/jira/browse/SANDBOX-464
Project: Commons Sandbox
Issue Type: Task
Components: BeanUtils2
Affects Versions: Nightly Builds
Reporter: Benedikt Ritter
Attachments: Commons-Sandbox-464-Get.patch
BeanUtils 1 has the functionality to interpret property paths. For example
the call:
{code:java}
BeanUtils.getProperty(adress.street, person);
{code}
would first get the address of the given person and then get the street from
the address and return it. The same is possible for list and mapped
properties.
In order to be supplement for BeanUtils1, we need to implement this in
BeanUtils2.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Updated] (SANDBOX-471) Typos and codestyle violations
[ https://issues.apache.org/jira/browse/SANDBOX-471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-471: --- Attachment: Commons-Sandbox-471#2.patch Typos and codestyle violations -- Key: SANDBOX-471 URL: https://issues.apache.org/jira/browse/SANDBOX-471 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Priority: Minor Attachments: Commons-Sandbox-471#2.patch, Commons-Sandbox-471.patch The GetPropertyTestCase contained two typos in method names and some codestyle violations in regards to Maven codestyle. This patch corrected the typos and reformatted the code. (Since it's a small and minor priority patch I hope it's ok to do both in one patch?!) -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (SANDBOX-471) Typos and codestyle violations
[ https://issues.apache.org/jira/browse/SANDBOX-471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13937295#comment-13937295 ] Andre Diermann commented on SANDBOX-471: Very strange! I am using the codestyle from maven [1], where SPACE_WITHIN_ANNOTATION_PARENTHESES is set to true. When I enable/disable this option in the preferences of IntelliJ the preview shows the correct behavior. But when I add the spaces in code and do code formatting the spaces are removed?!?! So you are totally right! Regarding the white spaces rule from [2] there should be spaces between the braces. I really don't know why IntelliJ removes the spaces. Big sorry for inconvenience! I will update the patch ASAP. [1] https://maven.apache.org/developers/maven-idea-codestyle.xml [2] https://maven.apache.org/developers/conventions/code.html Typos and codestyle violations -- Key: SANDBOX-471 URL: https://issues.apache.org/jira/browse/SANDBOX-471 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Priority: Minor Attachments: Commons-Sandbox-471.patch The GetPropertyTestCase contained two typos in method names and some codestyle violations in regards to Maven codestyle. This patch corrected the typos and reformatted the code. (Since it's a small and minor priority patch I hope it's ok to do both in one patch?!) -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (SANDBOX-464) Add support for interpreting property expressions
[
https://issues.apache.org/jira/browse/SANDBOX-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13936138#comment-13936138
]
Andre Diermann commented on SANDBOX-464:
Hello,
I have one question regarding the public API of BU2. Would it be ok, to add
another method named getSimple(), which returns a BeanAccessor for a simple
property where get() could be used to return a BeanAccessor for property
expressions?
Background:
Currently a BeanAccessor provides:
* get()
* getIndexed()
* getMapped()
...where get() is an equivalent to BU1's getSimple(), see [1].
When get() should, like requested in above's issue description, also deal with
property expressions it would give the API user a better control whether he/she
wants to get only a simple property or he/she want's to resolve a complete
property path. From an implementation point of view this would also help to
keep changes small, because the current get() method would become getSimple()
and the implementation of get would be:
public BeanAccessor? get( String propertyName )
{
return PropertyInterpreter.interpret( this, propertyName );
}
For me these are two advantages, a more explicit API usage for the user and a
simple implementation. But since it would effect the public API I want to check
with you guys first.
What do you think?
BR
André
[1]
http://commons.apache.org/proper/commons-beanutils/javadocs/v1.9.1/apidocs/index.html
Add support for interpreting property expressions
-
Key: SANDBOX-464
URL: https://issues.apache.org/jira/browse/SANDBOX-464
Project: Commons Sandbox
Issue Type: Task
Components: BeanUtils2
Affects Versions: Nightly Builds
Reporter: Benedikt Ritter
BeanUtils 1 has the functionality to interpret property paths. For example
the call:
{code:java}
BeanUtils.getProperty(adress.street, person);
{code}
would first get the address of the given person and then get the street from
the address and return it. The same is possible for list and mapped
properties.
In order to be supplement for BeanUtils1, we need to implement this in
BeanUtils2.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Comment Edited] (SANDBOX-464) Add support for interpreting property expressions
[
https://issues.apache.org/jira/browse/SANDBOX-464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13936138#comment-13936138
]
Andre Diermann edited comment on SANDBOX-464 at 3/15/14 11:03 AM:
--
Hello,
I have one question regarding the public API of BU2. Would it be ok, to add
another method named getSimple(), which returns a BeanAccessor for a simple
property where get() could be used to return a BeanAccessor for property
expressions?
Background:
Currently a BeanAccessor provides:
* get()
* getIndexed()
* getMapped()
...where get() is an equivalent to BU1's getSimple(), see [1].
When get() should, like requested in above's issue description, also deal with
property expressions it would give the API user a better control whether he/she
wants to get only a simple property or he/she want's to resolve a complete
property path. From an implementation point of view this would also help to
keep changes small, because the current get() method would become getSimple()
and the implementation of get would be:
public BeanAccessor? get( String propertyName )
{
return PropertyInterpreter.interpret( this, propertyName );
}
For me these are two advantages, a more explicit API usage for the user and a
simple implementation. But since it would affect the public API I want to check
with you guys first.
What do you think?
BR
André
[1]
http://commons.apache.org/proper/commons-beanutils/javadocs/v1.9.1/apidocs/index.html
was (Author: andd):
Hello,
I have one question regarding the public API of BU2. Would it be ok, to add
another method named getSimple(), which returns a BeanAccessor for a simple
property where get() could be used to return a BeanAccessor for property
expressions?
Background:
Currently a BeanAccessor provides:
* get()
* getIndexed()
* getMapped()
...where get() is an equivalent to BU1's getSimple(), see [1].
When get() should, like requested in above's issue description, also deal with
property expressions it would give the API user a better control whether he/she
wants to get only a simple property or he/she want's to resolve a complete
property path. From an implementation point of view this would also help to
keep changes small, because the current get() method would become getSimple()
and the implementation of get would be:
public BeanAccessor? get( String propertyName )
{
return PropertyInterpreter.interpret( this, propertyName );
}
For me these are two advantages, a more explicit API usage for the user and a
simple implementation. But since it would effect the public API I want to check
with you guys first.
What do you think?
BR
André
[1]
http://commons.apache.org/proper/commons-beanutils/javadocs/v1.9.1/apidocs/index.html
Add support for interpreting property expressions
-
Key: SANDBOX-464
URL: https://issues.apache.org/jira/browse/SANDBOX-464
Project: Commons Sandbox
Issue Type: Task
Components: BeanUtils2
Affects Versions: Nightly Builds
Reporter: Benedikt Ritter
BeanUtils 1 has the functionality to interpret property paths. For example
the call:
{code:java}
BeanUtils.getProperty(adress.street, person);
{code}
would first get the address of the given person and then get the street from
the address and return it. The same is possible for list and mapped
properties.
In order to be supplement for BeanUtils1, we need to implement this in
BeanUtils2.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
[jira] [Created] (SANDBOX-471) Typos and codestyle violations
Andre Diermann created SANDBOX-471: -- Summary: Typos and codestyle violations Key: SANDBOX-471 URL: https://issues.apache.org/jira/browse/SANDBOX-471 Project: Commons Sandbox Issue Type: Bug Components: BeanUtils2 Reporter: Andre Diermann Priority: Minor The GetPropertyTestCase contained two typos in method names and some codestyle violations in regards to Maven codestyle. This patch corrected the typos and reformatted the code. (Since it's a small and minor priority patch I hope it's ok to do both in one patch?!) -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Updated] (SANDBOX-471) Typos and codestyle violations
[ https://issues.apache.org/jira/browse/SANDBOX-471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-471: --- Attachment: Commons-Sandbox-471.patch Typos and codestyle violations -- Key: SANDBOX-471 URL: https://issues.apache.org/jira/browse/SANDBOX-471 Project: Commons Sandbox Issue Type: Bug Components: BeanUtils2 Reporter: Andre Diermann Priority: Minor Attachments: Commons-Sandbox-471.patch The GetPropertyTestCase contained two typos in method names and some codestyle violations in regards to Maven codestyle. This patch corrected the typos and reformatted the code. (Since it's a small and minor priority patch I hope it's ok to do both in one patch?!) -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Updated] (SANDBOX-471) Typos and codestyle violations
[ https://issues.apache.org/jira/browse/SANDBOX-471?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-471: --- Issue Type: Improvement (was: Bug) Typos and codestyle violations -- Key: SANDBOX-471 URL: https://issues.apache.org/jira/browse/SANDBOX-471 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Priority: Minor Attachments: Commons-Sandbox-471.patch The GetPropertyTestCase contained two typos in method names and some codestyle violations in regards to Maven codestyle. This patch corrected the typos and reformatted the code. (Since it's a small and minor priority patch I hope it's ok to do both in one patch?!) -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13932086#comment-13932086 ] Andre Diermann commented on SANDBOX-463: Many thanks for reviewing the patch and your feedback! 1. You are right, I will extend the test cases to contain negative tests as well! 2. The method name is a result of using the GOF Interpreter Pattern, where a (bean) property is an expression, a BeanAccessor is a context which is required for interpretation and the PropertyInterpreter is the client which brings all together. See [1] for instance. So the intention of this method is not to interpret the BeanAccesor, but to interpret the Expression with the use of the BeanAccesor. If you search for some applications of the Interpreter Pattern you will find that it's common use to pass in simple objects as context without wrapping it into a dedicated context class (which would be necessary if you need more than one object for interpretation). When using patterns like this, I like to keep their terminology to express the usage of a pattern explicitly. I think it's more convenient. So when another developer, which is aware of design patterns, will see an Expression-interface with one solely interpret-method it's more obvious what's going on. That's why I think it would be nice to keep the pattern's terminology. But I can change the method name, if you think it fits better to the domain of BU2. 3. H... Please help me what you intent here, I can't see what you mean. :-( [1] http://www.oodesign.com/interpreter-pattern.html Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463#2.patch, Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13932199#comment-13932199 ] Andre Diermann commented on SANDBOX-463: #3 now contains more tests (especially testing invalid expressions) as well as pre-compiled regex-patterns as static final members. In addition to that I set the field propertyName to final (since it issn't foreseen to be changed) and resolved some last CheckStyle violations. Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463#2.patch, Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Updated] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-463: --- Attachment: Commons-Sandbox-463#3.patch Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463#2.patch, Commons-Sandbox-463#3.patch, Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13930564#comment-13930564 ] Andre Diermann commented on SANDBOX-463: Thank you Bernd. I think I should rework the syntax for valid map keys and make it less strict! I am currently evaluating OGNL - unfortunately I was not aware of this before : Do you have a good picture of OGNL? What do you think? Maybe it already does what we are looking for? Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463#2.patch, Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Created] (SANDBOX-463) Implementation of property paths
Andre Diermann created SANDBOX-463: -- Summary: Implementation of property paths Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Updated] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-463: --- Attachment: Commons-Sandbox-463.patch Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Updated] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-463: --- Attachment: (was: Commons-Sandbox-463.patch) Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Updated] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-463: --- Attachment: Commons-Sandbox-463.patch Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- 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-tabpanelfocusedCommentId=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] [Updated] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-463: --- Attachment: Commons-Sandbox-463#2.patch Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463#2.patch, Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13924870#comment-13924870 ] Andre Diermann commented on SANDBOX-463: Hello Benedikt, many thanks for reviewing the patch and for your feedback! :) In Commons-Sandbox-463#2.patch you will find your proposed improvements. I furthermore resolved 13 checkstyle violations (thanks to your fix of mvn site :)) and changed the order of two method declarations. One remark: I am not totally sure about the correct syntax of keys for mapped properties. From the BU1 code it seems that every character is valid. But I specified it as ([a-z]+[A-Za-z]*)\\(([A-Za-z0-9\\s]+)\\), which means only [A-Za-z0-9\\s]+ is valid as key names, which is more restrictive. I feel that this might be an issue in the future. What do you think? BR André [2] see next() in http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/main/java/org/apache/commons/beanutils/expression/DefaultResolver.java?revision=1454597view=markup Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463#2.patch, Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Comment Edited] (SANDBOX-463) Implementation of property paths
[ https://issues.apache.org/jira/browse/SANDBOX-463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13924870#comment-13924870 ] Andre Diermann edited comment on SANDBOX-463 at 3/8/14 2:33 PM: Hello Benedikt, many thanks for reviewing the patch and for your feedback! :) In Commons-Sandbox-463#2.patch you will find your proposed improvements. I furthermore resolved 13 checkstyle violations (thanks to your fix of mvn site :)) and changed the order of two method declarations. One remark: I am not totally sure about the correct syntax of keys for mapped properties. From the BU1 code it seems that every character is valid [1]. But I specified it as ([a-z]+[A-Za-z]*)\\(([A-Za-z0-9\\s]+)\\), which means only [A-Za-z0-9\\s]+ is valid as key names, which is more restrictive. I feel that this might be an issue in the future. What do you think? BR André [1] see next() in http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/main/java/org/apache/commons/beanutils/expression/DefaultResolver.java?revision=1454597view=markup was (Author: andd): Hello Benedikt, many thanks for reviewing the patch and for your feedback! :) In Commons-Sandbox-463#2.patch you will find your proposed improvements. I furthermore resolved 13 checkstyle violations (thanks to your fix of mvn site :)) and changed the order of two method declarations. One remark: I am not totally sure about the correct syntax of keys for mapped properties. From the BU1 code it seems that every character is valid. But I specified it as ([a-z]+[A-Za-z]*)\\(([A-Za-z0-9\\s]+)\\), which means only [A-Za-z0-9\\s]+ is valid as key names, which is more restrictive. I feel that this might be an issue in the future. What do you think? BR André [2] see next() in http://svn.apache.org/viewvc/commons/proper/beanutils/trunk/src/main/java/org/apache/commons/beanutils/expression/DefaultResolver.java?revision=1454597view=markup Implementation of property paths - Key: SANDBOX-463 URL: https://issues.apache.org/jira/browse/SANDBOX-463 Project: Commons Sandbox Issue Type: Improvement Components: BeanUtils2 Reporter: Andre Diermann Attachments: Commons-Sandbox-463#2.patch, Commons-Sandbox-463.patch In [1] Benedikt Ritter mentioned the lag of BU1's property path for BU2. The attached patch contains an implementation of a PropertyInterpreter - following the GOF Interpreter Pattern - which is able to interpret simple/index/mapped/nested properties and any combination of it. The result is a BeanAccessor. The idea is to use the PropertyInterpreter inside of a BeanAccessor implementation. The patch also contains a corresponding test case. Example: on( anyBean ).set( contact[15].address(home).street ).with( Example Street 1000 ); [1] http://markmail.org/message/pv2qpr7em4ftnulj -- 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-tabpanelfocusedCommentId=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] [Resolved] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
[ https://issues.apache.org/jira/browse/COMMONSSITE-78?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann resolved COMMONSSITE-78. --- Resolution: Fixed Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to align to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
[ https://issues.apache.org/jira/browse/COMMONSSITE-78?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13908094#comment-13908094 ] Andre Diermann commented on COMMONSSITE-78: --- Yeah, I changed the issue to resolved. Unfortunately I am not aware of the version which fixes the issue. If this information is important please tell me so that I can update it. (Or maybe update by yourself if this is possible somehow :)) Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to align to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- 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-tabpanelfocusedCommentId=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] [Updated] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
[ https://issues.apache.org/jira/browse/COMMONSSITE-78?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated COMMONSSITE-78: -- Attachment: Issue#1.PNG Issue#2.PNG Issue#3.PNG Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to aligned to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Created] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
Andre Diermann created COMMONSSITE-78: - Summary: Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to aligned to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Updated] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
[ https://issues.apache.org/jira/browse/COMMONSSITE-78?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated COMMONSSITE-78: -- Description: When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to aligne to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. was: When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to aligned to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to aligne to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Updated] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
[ https://issues.apache.org/jira/browse/COMMONSSITE-78?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated COMMONSSITE-78: -- Description: When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to align to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. was: When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to aligne to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to align to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
[ https://issues.apache.org/jira/browse/COMMONSSITE-78?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13905741#comment-13905741 ] Andre Diermann commented on COMMONSSITE-78: --- The issue is reproduceable on Chrome and MacOSX 10.9 as well. ... wait, have you fixed it in this second? Wail typing this comment it seems to be fixed now :) Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to align to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Comment Edited] (COMMONSSITE-78) Alignment Issues on Commons Welcome Page
[ https://issues.apache.org/jira/browse/COMMONSSITE-78?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13905741#comment-13905741 ] Andre Diermann edited comment on COMMONSSITE-78 at 2/19/14 5:43 PM: The issue is reproduceable on Chrome and MacOSX 10.9 as well. ... wait, have you fixed it in this second? While typing this comment it seems to be fixed now :) was (Author: andd): The issue is reproduceable on Chrome and MacOSX 10.9 as well. ... wait, have you fixed it in this second? Wail typing this comment it seems to be fixed now :) Alignment Issues on Commons Welcome Page Key: COMMONSSITE-78 URL: https://issues.apache.org/jira/browse/COMMONSSITE-78 Project: Commons All Issue Type: Bug Components: Commons Skin Environment: Chrome Version 32.0.1700.107 Windows 7 Reporter: Andre Diermann Attachments: Issue#1.PNG, Issue#2.PNG, Issue#3.PNG When browsing http://commons.apache.org/ with the new skin, I am encountering several issues regarding the alignment of some page items: #1: The publish and version information in the navigation bar seems to align to top instead of beeing centered vertically like the other items within the navigation bar. #2: The left-hand side menu is centered vertically which makes it somehow invisible. The user has to scroll down on larger pages such as the above mentioned start page. #3: The footer seems to be not formatted correctly when compared to the footer of http://commons.apache.org/proper/commons-weaver/ See attached screenshots. -- 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-tabpanelfocusedCommentId=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] [Updated] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-462: --- Attachment: Commons-BeanUtils2-462#1.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, 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-tabpanelfocusedCommentId=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] [Updated] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-462: --- Attachment: (was: Commons-BeanUtils2-462.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.1.5#6160)
[jira] [Created] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
Andre Diermann created SANDBOX-462: -- Summary: 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 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. 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). - 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] [Updated] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-462: --- Attachment: Commons-BeanUtils2-462.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.patch 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. 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). - 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] [Updated] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-462: --- Description: 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). - 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. was: 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. 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). - 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. 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). - 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] [Updated] (SANDBOX-462) Refactoring of AccessibleObjectsRegistry
[ https://issues.apache.org/jira/browse/SANDBOX-462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andre Diermann updated SANDBOX-462: --- Description: 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. was: 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). - 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. 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)
