You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Benedikt Ritter (Created) (JIRA)" <ji...@apache.org> on 2012/02/25 16:37:48 UTC
[jira] [Created] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
[BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
--------------------------------------------------------------------------------------------------------
Key: SANDBOX-397
URL: https://issues.apache.org/jira/browse/SANDBOX-397
Project: Commons Sandbox
Issue Type: Task
Components: BeanUtils2
Affects Versions: Nightly Builds
Reporter: Benedikt Ritter
At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Simone Tripodi (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217086#comment-13217086 ]
Simone Tripodi commented on SANDBOX-397:
----------------------------------------
I still haven't had the time to take a look at your patches, we didn't even agree on the proposed design that you already tracked all the roadmap? What if I don't agree on one of the the proposed modifications?
_I think it is the right thing to do_ doesn't mean that *it is*.
I'll let you know as soon as I get the chance to have a look at your patches, and provide you feedbacks.
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Attachments: SANDBOX-397.txt, SANDBOX-397_SRP.txt, SANDBOX-397_SRPv2.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Benedikt Ritter (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benedikt Ritter updated SANDBOX-397:
------------------------------------
Attachment: SANDBOX-397_SRP.txt
I have created another patch, that builds upon the first one. I think it is not a responsibility of a {{DefaultBeanAccessor}} to figure out, whether or not properties are available or not. So the private methods from the first patch are a violation of SRP [1].
For this reason I created a new class, that is reponsible for handling properties of a class: {{Properties<B>}}. I think this is a nice and clean solution to the SRP violation. However the patch is incomplete, because several unit tests fail (for details see below). I uploaded the patch anyway, because I want to know, what you think about the hole idea, before I start to fix the issues raised by the new class.
The reason for the failing tests is, that {{DefaultBeanAccessor}} creates a {{Properties}} object in its constructor using the bean object passed in. A {{DefaultBeanAccessor}} can be created in several ways:
* {{BeanUtils.on(B bean)}}
* {{ArgumentsAccessor.with(...)}}
If in the latter case, a method is invoked that returns void, instantiation of {{DefaultBeanAccessor}} will fail, because the constructor tries to call {{getClass()}} on {{null}}.
I think this is not a problem of the newly invented {{Properties<B>}} class, but of {{ArgumentAccessor.with(...)}}. In other words, this was broken before but we had no tests for this case.
A solution could be to write a Special Case [2] implementation of {{BeanAccessor}} for this case ({{NullBeanAccessor}}) that will be returned by {{ArgumentsAccessor.with(...)}} if a method returns void.
What do you think?
All the best,
Benedikt
[1] http://en.wikipedia.org/wiki/Single_responsibility_principle
[2] http://martinfowler.com/eaaCatalog/specialCase.html
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Attachments: SANDBOX-397.txt, SANDBOX-397_SRP.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Simone Tripodi resolved SANDBOX-397.
------------------------------------
Resolution: Fixed
The idea was generally *good* - the {{Properties}} class helped to maintain code simple and straightforward - what I did to improve the proposed patch:
* fixed s/Descirptor/Descriptor typos ;)
* s/Properties/DefaultBeanProperties
* dropped useless {{else}} branches;
* introduced a static method to check not null methods (DRY principle);
* extracted an interface from {{DefaultBeanProperties}}, {{BeanProperties}};
* moved the {{of(Class)}} method from {{Properties}} to {{ClassAccessor}}
* moved {{is(Readable|Writable)}} from {{BeanAccessor}} to {{BeanProperties}} it is more canonical (and removed useless {{else}} branches here too)
* adapted {{is(Readable|Writable)TestCase}} and {{VoidMethodsTestCase}}
please have a look at r1348122 to see how APIs look now.
I am not totally satisfied about methods such as {{get(Indexed)(Read|Write)PropertyDescriptor()}}, I think I'll replace them in order to return directly the Reader/Writer methods.
nice job, and thanks for contributing!
-Simo
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Assignee: Simone Tripodi
> Attachments: SANDBOX-397.txt, SANDBOX-397_SRP.txt, SANDBOX-397_SRPv2.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Benedikt Ritter (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13293845#comment-13293845 ]
Benedikt Ritter commented on SANDBOX-397:
-----------------------------------------
Hi Simo,
I'm very happy to hear, that you liked my idea. Many thanks for your improvements. Making {{BeanProperties}} publicly available is a good idea. Here are some comments regarding your commit:
* Usually I try to avoid long return statements like the ones introduced through {{checkMethod}} in {{DefaultBeanProperties}}. To much inlining can be very confusing. However, since there isn't to much logic in those methods, I guess we can leave it as it is.
* You should have a look at the extracted {{BeanProperties}} interface. I guess your IDE refactoring created some public modifiers and some abstract method declarations, that we can get rid of.
* I'm not happy about those methods in {{BeanProperties}} either, but at the time I didn't have a better solution. Feel free to change what ever you like (like you already did ;) )
regards,
Benedikt
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Assignee: Simone Tripodi
> Attachments: SANDBOX-397.txt, SANDBOX-397_SRP.txt, SANDBOX-397_SRPv2.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Benedikt Ritter (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benedikt Ritter updated SANDBOX-397:
------------------------------------
Attachment: SANDBOX-397.txt
I have created a patch for this issue. Because there was a good amount of identical code, I wrapped the retrieval of {{PropertyDescriptors}} into private methods.
I also added some notNull checks where ever property names a passed in. Test cases have been modified as well.
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Attachments: SANDBOX-397.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Benedikt Ritter (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13216947#comment-13216947 ]
Benedikt Ritter edited comment on SANDBOX-397 at 2/26/12 9:59 PM:
------------------------------------------------------------------
I've thought about the {{Properties}} class some more and I think, that it is the right thing to do. So I've created a patch for SANDBOX-399 that will fix the problems that {{Properties}} caused. I've done some more clean up and I was able to eliminate the dependency between {{DefaultBeanAccessor}} and {{PropertiesRegistry}}. {{PropertiesRegistry}} is on a lower abstraction level than {{DefautlBeanAccessor}} so I think that is a good thing :)
If SANDBOX-399 is applied first, all test will pass when SANDBOX-397-SRPv2.txt is applied.
Regards,
Benedikt
was (Author: britter):
I've thought about the {{Properties}} class some more and I think, that it is the right thing to do. So I've created a patch for SANDBOX-399 that will fix the problems that {{Properties}} caused. I've done some more clean up and I was able to eliminate the dependency between {{DefaultBeanAccessor}} and {{PropertiesRegistry}}. {{PropertiesRegistry}} is on a lower abstraction level than {{DefautlBeanAccessor}} so I think that is a good thing :)
If SANDBOX-399 is applied first, all test will pass when SANDBOX-398-SRPv2.txt is applied.
Regards,
Benedikt
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Attachments: SANDBOX-397.txt, SANDBOX-397_SRP.txt, SANDBOX-397_SRPv2.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Benedikt Ritter (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benedikt Ritter updated SANDBOX-397:
------------------------------------
Attachment: SANDBOX-397_SRPv2.txt
I've thought about the {{Properties}} class some more and I think, that it is the right thing to do. So I've created a patch for SANDBOX-399 that will fix the problems that {{Properties}} caused. I've done some more clean up and I was able to eliminate the dependency between {{DefaultBeanAccessor}} and {{PropertiesRegistry}}. {{PropertiesRegistry}} is on a lower abstraction level than {{DefautlBeanAccessor}} so I think that is a good thing :)
If SANDBOX-399 is applied first, all test will pass when SANDBOX-398-SRPv2.txt is applied.
Regards,
Benedikt
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Attachments: SANDBOX-397.txt, SANDBOX-397_SRP.txt, SANDBOX-397_SRPv2.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (SANDBOX-397) [BeanUtils2] Replace
NullPointerExceptions been thrown in DefaultBeanAccessor with
NoSuchMethodEceptions
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SANDBOX-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13293876#comment-13293876 ]
Simone Tripodi commented on SANDBOX-397:
----------------------------------------
guten abend Bene,
{quote}
Usually I try to avoid long return statements like the ones introduced through {{checkMethod}} in {{DefaultBeanProperties}}. To much inlining can be very confusing. However, since there isn't to much logic in those methods, I guess we can leave it as it is.
{quote}
I am not sure I understood what you are referring - {{DefaultBeanProperties#checkMethod()}} is similar to {{Assertions#checkNotNull}} but they throw different exceptions...
{quote}
You should have a look at the extracted {{BeanProperties}} interface. I guess your IDE refactoring created some public modifiers and some abstract method declarations, that we can get rid of.
{quote}
hehehe yup perfectly right - I have been too lazy this time, thanks for pointing!
{quote}
I'm not happy about those methods in {{BeanProperties}} either, but at the time I didn't have a better solution. Feel free to change what ever you like (like you already did ;) )
{quote}
nice to hear we found anyway an agreement!!
looking forward to apply more patches from you, alles gute!
> [BeanUtils2] Replace NullPointerExceptions been thrown in DefaultBeanAccessor with NoSuchMethodEceptions
> --------------------------------------------------------------------------------------------------------
>
> Key: SANDBOX-397
> URL: https://issues.apache.org/jira/browse/SANDBOX-397
> Project: Commons Sandbox
> Issue Type: Task
> Components: BeanUtils2
> Affects Versions: Nightly Builds
> Reporter: Benedikt Ritter
> Assignee: Simone Tripodi
> Attachments: SANDBOX-397.txt, SANDBOX-397_SRP.txt, SANDBOX-397_SRPv2.txt
>
>
> At the moment, methods in {{DefaultBeanAccessor}} throw a {{NullPointerException}}, if no {{PropertyDescriptor}} for a given property name can be retrieved. As discussed on the ML (see http://markmail.org/thread/zlehclmybp5xgn5n) this behavior should be changed to throwing {{NoSuchMethodException}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira