[+ Brandon]
Sherman,
Here are some review comments on "take1"
Overall, looks good.
This low-level API will be annoying to use,
but that's mostly unavoidable.
----
typos: @linke sinze
----
source files contain TABs, no longer permitted
----
<tt> is denigrated in favor of @code
----
I don't think you really meant to include ./Flush.java in the webrev
----
{...@link Deflater.SYNC_FLUSH}
=>
{...@link Deflater#SYNC_FLUSH}
----
Please use a Google copyright in InflateIn_DeflateOut
----
Martin
On Tue, Sep 22, 2009 at 13:03, Xueming Shen <xueming.s...@sun.com> wrote:
>
> Thanks Alan!
>
> The webrev has been updated accordingly to address your comments (use
> syncFlush instead of
> doSyncFlush and those suggestions for Deflater.java).
>
> http://cr.openjdk.java.net/~sherman/zipflush/webrev
>
> Sherman
>
> PS. There was a "take2" for DOS that I think might be more consistent with
> the existing APIs, but it might be the
> time to focus on one approach. If you're interested the "take2" is at
>
> http://cr.openjdk.java.net/~sherman/zipflush/webrev.take2/src/share/classes/java/util/zip/DeflaterOutputStream.java.sdiff.html
>
> Alan Bateman wrote:
>>
>> Welcome back (for some reason I thought you were gone for two weeks).
>>
>> It would be best to send the proposal to the mailing list so that others
>> can comment.
>>
>> Personally, I don't like exposing the flush mode to subclasses but you are
>> right that it is more consistent with the original design. If this approach
>> is chosen then I would suggest that the flush implementation make a copy of
>> the flush mode before testing and using it. Also, the new constructors will
>> need to say that they initialize the flush mode based on the parameter
>> (since it can change on the fly). BTW: What is the reason for renaming the
>> parameter to flushDef. I prefer "syncFlush" over "doSyncFlush".
>>
>> I'm happy with Deflater. In the description of SYNC_FLUSH I would suggest
>> changing the "," into a ";" or else make it into two sentences. In the new
>> deflate method the description of NO_FLUSH needs a comma after "accumulate"
>> for it to flow well. Similar issue in the SYNC_FLUSH description where you
>> need a comma after "is flushed". There is also a typo [ comparssed :-) ]
>>
>> -Alan.
>>
>>
>
>
