You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Adrian Cumiskey (Issue Comment Edited) (JIRA)" <ji...@apache.org> on 2011/10/28 06:24:32 UTC
[jira] [Issue Comment Edited] (OGNL-31) [PATCH] Some CPD fixes
around ASTMethod and ASTStaticMethod.
[ https://issues.apache.org/jira/browse/OGNL-31?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13137989#comment-13137989 ]
Adrian Cumiskey edited comment on OGNL-31 at 10/28/11 4:22 AM:
---------------------------------------------------------------
@Maurizio Cucchiara
I believe you are making reference to a different patch contribution https://issues.apache.org/jira/browse/OGNL-23 which Simone applied.
I have attached a replacement patch which I believe will address your concerns. I have also added the test case you described to the patch.
Although harmless, the compiler caching implementation wasn't ideal or necessary so I have removed it. The context is only passed to the getCompiler() method in order for the OgnlRuntime to perform its class resolving. Once this has been achieved the first time the context and class resolving is unnecessary on subsequent calls so we can just return the valid _compiler reference that is held.
@Adrian Crum, Simone Tripodi
I have removed both Luke and Drew as @authors in the newly introduced class ASTMethodUtil. I decided to provide a default private constructor as this class should never be instantiated and also made both the class and its methods package private as they will (and should) only ever be accessed by ASTMethod and ASTStaticMethod. I was slightly tempted to make ASTMethodUtil a base class but its methods have no dependencies on any of the instance variables in either ASTMethod or ASTStaticMethod so I decided to leave it as it is. Wishing you all a nice weekend.
Cheers, Adrian.
was (Author: acumiskey):
@Maurizio Cucchiara
I believe you are making reference to a different patch contribution https://issues.apache.org/jira/browse/OGNL-23 which Simone applied.
I have attached a replacement patch which
I believe will address your concerns. I have also added the test case you described to the patch.
Although harmless, the compiler caching implementation wasn't ideal or necessary so I have removed it. The context is only passed to the getCompiler() method in order for the OgnlRuntime to perform its class resolving. Once this has been achieved the first time the context and class resolving is unnecessary on subsequent calls so we can just return the valid _compiler reference that is held.
@Adrian Crum, Simone Tripodi
I have removed both Luke and Drew as @authors in the newly introduced class ASTMethodUtil. I decided to provide a default private constructor as this class should never be instantiated and also made both the class and its methods package private as they will (and should) only ever be accessed by ASTMethod and ASTStaticMethod. I was slightly tempted to make ASTMethodUtil a base class but its methods have no dependencies on any of the instance variables in either ASTMethod or ASTStaticMethod so I decided to leave it as it is. Wishing you all a nice weekend.
Cheers, Adrian.
> [PATCH] Some CPD fixes around ASTMethod and ASTStaticMethod.
> ------------------------------------------------------------
>
> Key: OGNL-31
> URL: https://issues.apache.org/jira/browse/OGNL-31
> Project: OGNL
> Issue Type: Improvement
> Reporter: Adrian Cumiskey
> Assignee: Maurizio Cucchiara
> Priority: Minor
> Attachments: patch-OGNL31-v2.txt, patch-OGNL31.txt
>
>
> I discovered a number of PMD CPD issues around ASTMethod and ASTStaticMethod class, there was quite a bit of duplication there.
> This patch comes courtesy of the wifi connection in my local laundry-mat here in Milwaukee. Its strange the things people will do to make their clothes wash more quickly :-).
> Cheers, Adrian.
--
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