[
https://issues.apache.org/jira/browse/JCR-3819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14172333#comment-14172333
]
BELUGA BEHR edited comment on JCR-3819 at 10/15/14 1:01 PM:
------------------------------------------------------------
Tobias,
Thank you for the prompt response and feedback. I did not mean for this to be
taken verbatim, but to spark some discussion. I'm interested in using JR but
thought the code could use some massaging because it's not very consistent and
has some areas of sub-optimal programming techniques. I simply meant to spark
some discussion and to allow for discretion on the part of you and your team.
* Name it what ever you would like, I just noticed instances where it was being
referred to directly as log.error() and MyClass.log.error(). I was trying to
make consistent
* Personal preference I guess, but I always use them in my professional work
just to simply enforce what is a good practice. Saves time in protecting
against common mistakes.
* I do not believe I changed the synchronization anywhere, if you're referring
to the Map, I simply moved the synchronization to the Map's initialization as
its JavaDoc recommends.
* I did not change inheritance. The class was already inheriting that
interface already indirectly from another one of its interfaces. Seemed
duplicative.
* The signature change was a performance fix. It was using a StringBuffer, I
changed it to StringBuilder to remove some of the overhead of needless
synchronization
* I change the code-flow to make some methods easier to read and restructured
to remove unnecessary code (overhead)
Thank you so much for your time into this project. Much appreciated.
was (Author: belugabehr):
Tobias,
Thank you for the prompt response and feedback. I did not mean for this to be
taken verbatim, but to spark some discussion. I'm interested in using JR but
thought the code could use some massaging because it's not very consistent and
has some area of sub-optimal programming techniques. I simply meant to spark
some discussion and to allow for discretion on the part of you and your team.
* Name it what ever you would like, I just noticed instances where it was being
referred to directly as log.error() and MyClass.log.error(). I was trying to
make consistent
* Personal preference I guess, but I always use them in my professional work
just to simply enforce what is a good practice. Saves time in protecting
against common mistakes.
* I do not believe I changed the synchronization anywhere, if you're referring
to the Map, I simply moved the synchronization to the Map's initialization as
its JavaDoc recommends.
* I did not change inheritance. The class was already inheriting that
interface already indirectly from another one of its interfaces. Seemed
duplicative.
* The signature change was a performance fix. It was using a StringBuffer, I
changed it to StringBuilder to remove some of the overhead of needless
synchronization
* I change the code-flow to make some methods easier to read and restructured
to remove unnecessary code (overhead)
Thank you so much for your time into this project. Much appreciated.
> General Code Review
> -------------------
>
> Key: JCR-3819
> URL: https://issues.apache.org/jira/browse/JCR-3819
> Project: Jackrabbit Content Repository
> Issue Type: Improvement
> Components: jackrabbit-core
> Affects Versions: 2.9
> Reporter: BELUGA BEHR
> Priority: Minor
> Fix For: 2.9.1
>
> Attachments: jcr_code_review.patch
>
>
> Hello,
> Attached I have made some minor clean-up, standardizing, and improvements.
> Please review. There should be no functional changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
