Otis,

the English class is in cvs, that's where i found it. It is also used by other test classes like TestTermVectors e.g.

The IOException was something where i wasn't sure how to process. I think you're right, the best idea would be to pop it up to the caller. Looking at the original code, the IOException wasn't caught in TermVectors constructor.

Sorry about the tabs, this are my settings in exlipse, inserting tabs instead of blanks.

Shall i create a new patch send it to the list, or do i have to create a new bugzilla issue for that. Is it possible to update attachments in bugzilla ? Don't think so.

regards
Bernhard

[EMAIL PROTECTED] wrote:

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=30736>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE.


http://issues.apache.org/bugzilla/show_bug.cgi?id=30736

[PATCH] to remove synchronized code from TermVectorsReader





------- Additional Comments From [EMAIL PROTECTED]  2004-08-19 11:47 -------
Bernhard,

Thanks for the patch. The unit test requires class o.a.lucene.util.English. This is not in CVS. Is this something that should be in the CVS? What is it?

I am also wondering about this piece of code:

-      termVectorsReader = new TermVectorsReader(cfsDir, segment, fieldInfos);
+       final Directory dir = cfsDir;
+       termVectorsLocal = new ThreadLocal() {
+               protected synchronized Object initialValue() {
+                       try {
+                               return new TermVectorsReader(dir, segment,
fieldInfos);
+                       } catch (IOException ioe) {
+                               ioe.printStackTrace();
+                               return null;
+                       }
+               }
+       };

Is is a good thing to 'eat' that IOException and quietly return null?  The
method where this code is, is already throwing IOException, so why not let the
IOException pop up?

Finally, it looks like diffs contain tabs.  Could you please change tabs to 2
spaces?

Thanks,
Otis

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]





---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to