Mailing-List: contact dev-help@lucene.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@lucene.apache.org
Date: Mon, 6 Jun 2011 22:18:59 +0000 (UTC)
From: "Steven Rowe (JIRA)" <jira@apache.org>
To: dev@lucene.apache.org
Message-ID: 
 <129562057.2226.1307398739369.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Updated] (LUCENE-1736) DateTools.java general improvements
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


     [ https://issues.apache.org/jira/browse/LUCENE-1736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Steven Rowe updated LUCENE-1736:
--------------------------------

    Attachment: LUCENE-1736.patch

David, this is your patch with a CHANGES.txt entry and a couple of comments added ("for javadocs" next to the two imports that are javadocs-only; and formatLen spelled out over the shared format string).

Nice improvements.  All tests pass.

I plan on committing shortly.

> DateTools.java general improvements
> -----------------------------------
>
>                 Key: LUCENE-1736
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1736
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: core/index
>    Affects Versions: 2.9
>            Reporter: David Smiley
>            Assignee: Steven Rowe
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: LUCENE-1736.patch, LUCENE-1736_DateTools_improvements.patch, cleanerDateTools.patch
>
>
> Applying the attached patch shows the improvements to DateTools.java that I think should be done. All logic that does anything at all is moved to instance methods of the inner class Resolution. I argue this is more object-oriented.
> 1. In cases where Resolution is an argument to the method, I can simply invoke the appropriate call on the Resolution object. Formerly there was a big branch if/else.
> 2. Instead of "synchronized" being used seemingly everywhere, synchronized is used to sync on the object that is not threadsafe, be it a DateFormat or Calendar instance.
> 3. Since different DateFormat and Calendar instances are created per-Resolution, there is now less lock contention since threads using different resolutions will not use the same locks.
> 4. The old implementation of timeToString rounded the time before formatting it. That's unnecessary since the format only includes the resolution desired.
> 5. round() now uses a switch statement that benefits from fall-through (no break).
> Another debatable improvement that could be made is putting the resolution instances into an array indexed by format length. This would mean I could remove the switch in lookupResolutionByLength() and avoid the length constants there. Maybe that would be a bit too over-engineered when the switch is fine.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org