On 09/10/2009 04:51 PM, Xueming Shen wrote:
David M. Lloyd wrote:
On 09/10/2009 01:47 PM, Xueming Shen wrote:
I think we have enough discussion on this topic, it's time to make a
final decision. Seems like we are all happy on the
changes in Deflater and new DOS.flush(mode), the only difference is
whether or not we should change the existing
behavior of DOS.flush() should use Z_SYNC_FLUSH by default.
The question is, is there really such code out there? It can't really
have any useful semantics that I can see. My irrelevant vote is
firmly on "yes" for this.
- DML
-The question is, is there really such code out there?
I honestly don't know, this is why it is called "compatibility risk",
similar to the investment risk in the stock market, you
lose or gain only after you put the real money in:-) Though my feeling
is it is very "likely" this change is going to break
someone's code given DOS is designed/implemented to be easily used as a
"normal" OutputStream.
-It can't really have any useful semantics that I can see
DOS abstracts two functionalities, a deflater and an output stream, both
the deflater and output stream have their own
"flush" semantics. A flush on the deflater changes the final output,
degrades the compression, basically it changes the
"function" of the deflater. A flush on the OutputStream however is
supposed to push out the "buffered" output (accumulated
for whatever reason, mostly for the i/o performance), it does not change
the final result/output. So arguably it should not be
a too bad idea to also provide two types of "flush" to flush the
deflater and/or to flush the output stream, the existing
DOS.flush() "accidentally" provides the semantics of the outputstream's
flush(), with the help of the newly added flush(mode),
now the developer have the choice of doing whatever "flush" they want to
do.
This is not unusual in the JDK or elsewhere - that a stream calls the
underlying stream's flush() method I mean - and I think in the (unlikely)
event that someone wants to flush the underlying stream only, they could
simply call .flush() directly on that stream. And furthermore, if they
want to *prevent* the flush() method from calling the underlying flush(),
they can simply interpose a FlushIgnoringOutputStream which changes flush()
to a no-op and passes other calls through. So I'd say "no" to having a
separate flush method that does *that*.
That all said, I guess I should qualify my "yes" with the statement that if
the final decision were ultimately "no", it would be really, really trivial
for someone to subclass or wrap DOS with a version which does the full sync
flush instead of the pass-thru that your current webrev has. And come to
think of it, if you're going to provide your own deflater (which often
happens) you need to subclass DOS anyway to avoid leaking Deflaters on
stream close (since the "usesDefaultDeflater"-based auto-cleanup mechanism
is defeated in this case). So maybe it wouldn't be so bad to leave it as is...
- DML
