[ 
https://issues.apache.org/jira/browse/MAHOUT-788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13086101#comment-13086101
 ] 

Ted Dunning commented on MAHOUT-788:
------------------------------------

Lance,

That idiom is hideous.  catch Throwable is almost always a bug.

Better by far to say

{code}
   try {
      ... stuff ...
   } finally {
     try {
       writer.flush();
     } finally {
       if (shouldClose) {
         Closeables.closeQuietly(writer);
       }
     }
{code}

Pushing the flush into the first try clause might be reasonable as well.  If 
the first part blows out, we don't much care if we flush all data (that might 
be wrong anyway), but we probably do care to close files.  Thus,

{code}
   try {
     ... stuff ...
     writer.flush();
   } finally {
     if (shouldClose) {
       Closeables.closeQuietly(writer);
     }
   }
{code}
might be a better form all round.



> ClusterDumper is never flushing output stream
> ---------------------------------------------
>
>                 Key: MAHOUT-788
>                 URL: https://issues.apache.org/jira/browse/MAHOUT-788
>             Project: Mahout
>          Issue Type: Bug
>          Components: Clustering, Integration
>    Affects Versions: 0.6
>            Reporter: Jeff Hansen
>            Priority: Trivial
>             Fix For: 0.6
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> ClusterDumper utility never calls flush on the OutputStreamWriter.  As of 
> issue https://issues.apache.org/jira/browse/MAHOUT-679, the output stream is 
> never being closed when it defaults to System.out -- while that's a good 
> thing, it would be nice to flush the stream before exiting the program.  As 
> is, when I run cluster dumper with the -b (substring) option set to something 
> like 50, the stream never gets big enough to overflow the default buffer on 
> my machine, so I see no output.  Even when it does get big enough to overflow 
> the buffer, I still miss the last cluster's summary.  When the output is 
> written to a file, the close() method usually flushes the buffer by default, 
> but it shouldn't hurt to call the flush method either way -- therefore I'd 
> suggest adding in an unconditional call to writer.flush(); in the finally 
> block just before conditionally closing the writer. (line 199 in the 
> org.apache.mahout.utils.clustering.ClusterDumper.run(String[] args) method)
>     } finally {
>       writer.flush();
>       if (shouldClose) {
>         Closeables.closeQuietly(writer);
>       }
>     }

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

        

Reply via email to