Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> On Jul 23, 2019, at 11:58 PM, Alan Bateman  wrote:
> 
> On 23/07/2019 17:41, Brian Burkhalter wrote:
>> :
>> 
>> Here is an update which accounts for the foregoing comments.
>> 
>> http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.02/ 
>> 
>> 
> I looked at this version, and webrev-alt.03, but I think PS.write(byte[]) 
> will need further word smiting to properly explain the confusion on using 
> PrintStream vs. sub-classes of PrintStream that override this method (the 
> former does not throw IOE, the latter may). So I think the @apiNote will be 
> expanded further and the recommendation to use writeBytes(byte[]) or 
> write(byte[],0,len) instead should be dialed up. The @implSpec also needs to 
> be expanded as it doesn't make it clear that the default implementation (by 
> way of invoking super.write(byte[],int,int)) does not throw IOE. I'm also 
> wondering if PS.write(byte[]) should be deprecated. Hard to know how much 
> time to spend on this issue as it has existed since JDK 1.0.

Before addressing any of the above I wanted to note that in the fix for a very 
similar issue [1], where we added writeBytes(byte[]) to ByteArrayOutputStream, 
we did not do anything to its write(byte[]) method, i.e., it still does not 
override write(byte[]). I wonder whether doing the same here might simplify 
things.

If we are going to keep the write(byte[]) override, then perhaps it would be 
better to implement it to call out.write(buf,0,buf.length) directly.

Thanks,

Brian

[1] https://bugs.openjdk.java.net/browse/JDK-8180410

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Alan Bateman]

On 23/07/2019 17:41, Brian Burkhalter wrote:

:

Here is an update which accounts for the foregoing comments.

http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.02/

I looked at this version, and webrev-alt.03, but I think 
PS.write(byte[]) will need further word smiting to properly explain the 
confusion on using PrintStream vs. sub-classes of PrintStream that 
override this method (the former does not throw IOE, the latter may). So 
I think the @apiNote will be expanded further and the recommendation to 
use writeBytes(byte[]) or write(byte[],0,len) instead should be dialed 
up. The @implSpec also needs to be expanded as it doesn't make it clear 
that the default implementation (by way of invoking 
super.write(byte[],int,int)) does not throw IOE. I'm also wondering if 
PS.write(byte[]) should be deprecated. Hard to know how much time to 
spend on this issue as it has existed since JDK 1.0.


-Alan

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[David Holmes]

Sorry I was obviously missing the part of this thread where it was 
decided to not actually fix this as requested in the bug due to 
compatibility issues.


I agree with Brian that something must be added to the class-level 
javadoc explaining that due to a historical oversight there is one 
method that declares it throws IOException (and can actually throw it?).


I don't see why that method couldn't still honour the checkError 
protocol as well though.


David

On 24/07/2019 11:31 am, David Holmes wrote:

Jumping in here as this change is starting to really confuse me ...

On 24/07/2019 2:41 am, Brian Burkhalter wrote:



On Jul 23, 2019, at 8:27 AM, Brian Burkhalter 
 wrote:


On Jul 23, 2019, at 8:20 AM, Alan Bateman > wrote:


On 23/07/2019 16:08, Brian Burkhalter wrote:


I don’t see what you mean.
    @Override
    public void write(byte buf[]) throws IOException {
    super.write(buf);
    }
Should “trouble” be set and the IOE re-thrown?

super.write(byte[]) will invoke PrintStream overrides write(byte[], 
int, int) so IOE won't be thrown (and why the proposed change to the 
class description isn't right). The method description is probably 
the best place to describe the behavior.


Here is an update which accounts for the foregoing comments.

http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.02/


  609 @Override
  610 public void write(byte buf[]) throws IOException {
  611 super.write(buf, 0, buf.length);
  612 }

This is wrong! The whole point of this bug is to ensure that PrintStream 
methods honour its contract of NEVER throwing IOException. I mean that 
is what the bug synopsis and description are all about!


David
-


Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[David Holmes]

Jumping in here as this change is starting to really confuse me ...

On 24/07/2019 2:41 am, Brian Burkhalter wrote:




On Jul 23, 2019, at 8:27 AM, Brian Burkhalter  
wrote:


On Jul 23, 2019, at 8:20 AM, Alan Bateman mailto:alan.bate...@oracle.com>> wrote:

On 23/07/2019 16:08, Brian Burkhalter wrote:


I don’t see what you mean.
@Override
public void write(byte buf[]) throws IOException {
super.write(buf);
}
Should “trouble” be set and the IOE re-thrown?


super.write(byte[]) will invoke PrintStream overrides write(byte[], int, int) 
so IOE won't be thrown (and why the proposed change to the class description 
isn't right). The method description is probably the best place to describe the 
behavior.


Here is an update which accounts for the foregoing comments.

http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.02/


 609 @Override
 610 public void write(byte buf[]) throws IOException {
 611 super.write(buf, 0, buf.length);
 612 }

This is wrong! The whole point of this bug is to ensure that PrintStream 
methods honour its contract of NEVER throwing IOException. I mean that 
is what the bug synopsis and description are all about!


David
-


Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> On Jul 23, 2019, at 10:09 AM, Brian Burkhalter  
> wrote:
> 
>> You will want to add an @throws
> 
> Will do.
> 
>> The implSpec
>> —
>> * @implSpec
>> * The default implementation is equivalent to
>> * {@link java.io.FilterOutputStream#write(byte[],int,int)
>> * super.write(buf,0,buf.length)}.
>> *
>> ——
>> 
>> Not sure if “The default implementation” is the correct wording given it is 
>> not a default method (not sure what the norm is for describing the 
>> implementation in this case). Perhaps something like “This method is 
>> equivalent to calling ….”
> 
> I concur, that would be better.

Above changes made in [1].

Thanks,

Brian

[1] http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.03/

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

Hi Lance,

> On Jul 23, 2019, at 10:03 AM, Lance Andersen  
> wrote:
> 
> A couple of minor suggestions:
> 
> For
> 
> ———
> @Override
> public void write(byte buf[]) throws IOException {
> super.write(buf, 0, buf.length);
> }
> 
> ———
> 
> You will want to add an @throws

Will do.

> The implSpec
> —
>  * @implSpec
>  * The default implementation is equivalent to
>  * {@link java.io.FilterOutputStream#write(byte[],int,int)
>  * super.write(buf,0,buf.length)}.
>  *
> ——
> 
> Not sure if “The default implementation” is the correct wording given it is 
> not a default method (not sure what the norm is for describing the 
> implementation in this case). Perhaps something like “This method is 
> equivalent to calling ….”

I concur, that would be better.

> Where you have
> 
> 
>  *  Note that the bytes will be written as given; to write characters
>  * that will be translated according to the platform's default character
>  * encoding, use the {@code print(char[])} or {@code println(char[])}
>  * methods.
>  *
> ——
> 
> Did you consider this as a candidate for @apiNote as well?

No, because this wording exists elsewhere in the class but not as @apiNotes.

Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Lance Andersen]

Hi Brian

A couple of minor suggestions:

For

———
@Override
public void write(byte buf[]) throws IOException {
super.write(buf, 0, buf.length);
}

———

You will want to add an @throws

The implSpec
—
 * @implSpec
 * The default implementation is equivalent to
 * {@link java.io.FilterOutputStream#write(byte[],int,int)
 * super.write(buf,0,buf.length)}.
 *
——

Not sure if “The default implementation” is the correct wording given it is not 
a default method (not sure what the norm is for describing the implementation 
in this case). Perhaps something like “This method is equivalent to calling ….”


Where you have


 *  Note that the bytes will be written as given; to write characters
 * that will be translated according to the platform's default character
 * encoding, use the {@code print(char[])} or {@code println(char[])}
 * methods.
 *
——

Did you consider this as a candidate for @apiNote as well?


HTH

Lance
> On Jul 23, 2019, at 12:41 PM, Brian Burkhalter  
> wrote:
> 
> 
> 
>> On Jul 23, 2019, at 8:27 AM, Brian Burkhalter  
>> wrote:
>> 
>>> On Jul 23, 2019, at 8:20 AM, Alan Bateman >> > wrote:
>>> 
>>> On 23/07/2019 16:08, Brian Burkhalter wrote:
 
 I don’t see what you mean.
   @Override
   public void write(byte buf[]) throws IOException {
   super.write(buf);
   }
 Should “trouble” be set and the IOE re-thrown?
 
>>> super.write(byte[]) will invoke PrintStream overrides write(byte[], int, 
>>> int) so IOE won't be thrown (and why the proposed change to the class 
>>> description isn't right). The method description is probably the best place 
>>> to describe the behavior.
> 
> Here is an update which accounts for the foregoing comments.
> 
> http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.02/
> 
> Thanks,
> 
> Brian
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> On Jul 23, 2019, at 8:27 AM, Brian Burkhalter  
> wrote:
> 
>> On Jul 23, 2019, at 8:20 AM, Alan Bateman > > wrote:
>> 
>> On 23/07/2019 16:08, Brian Burkhalter wrote:
>>> 
>>> I don’t see what you mean.
>>>@Override
>>>public void write(byte buf[]) throws IOException {
>>>super.write(buf);
>>>}
>>> Should “trouble” be set and the IOE re-thrown?
>>> 
>> super.write(byte[]) will invoke PrintStream overrides write(byte[], int, 
>> int) so IOE won't be thrown (and why the proposed change to the class 
>> description isn't right). The method description is probably the best place 
>> to describe the behavior.

Here is an update which accounts for the foregoing comments.

http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.02/

Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> On Jul 23, 2019, at 8:20 AM, Alan Bateman  wrote:
> 
> On 23/07/2019 16:08, Brian Burkhalter wrote:
>> 
>> I don’t see what you mean.
>> @Override
>> public void write(byte buf[]) throws IOException {
>> super.write(buf);
>> }
>> Should “trouble” be set and the IOE re-thrown?
>> 
> super.write(byte[]) will invoke PrintStream overrides write(byte[], int, int) 
> so IOE won't be thrown (and why the proposed change to the class description 
> isn't right). The method description is probably the best place to describe 
> the behavior.

Oh, you are right. D’oh. Will update.

Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Alan Bateman]

On 23/07/2019 16:08, Brian Burkhalter wrote:


I don’t see what you mean.
 @Override
 public void write(byte buf[]) throws IOException {
 super.write(buf);
 }
Should “trouble” be set and the IOE re-thrown?

super.write(byte[]) will invoke PrintStream overrides write(byte[], int, 
int) so IOE won't be thrown (and why the proposed change to the class 
description isn't right). The method description is probably the best 
place to describe the behavior.


-Alan

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> On Jul 23, 2019, at 12:45 AM, Alan Bateman  wrote:
> 
> On 23/07/2019 01:01, Brian Burkhalter wrote:
>> :
>> This version [1] adds writeBytes() and overrides write(byte[]) without 
>> changing its behavior. The documentation of write(byte[]) points the user to 
>> writeBytes() and write(byte[],int.int).
>> 
> The overridden write(byte[]) declares that it throws IOE so it's source 
> compatible but it behaves like the other PS.write methods in that checkError 
> is used to check for an error rather than throwing IOE.

I don’t see what you mean.
@Override
public void write(byte buf[]) throws IOException {
super.write(buf);
}
Should “trouble” be set and the IOE re-thrown?

> This will need a bit more wording in the method description to make this 
> clearer. The change to the class description is confusing as it suggests that 
> the method throws IOE - maybe that change should be dropped to avoid trying 
> to explain the oddity in two places.

The current statement in the class doc without any change is however incorrect

"Unlike other output streams, a PrintStream never throws an IOException; […]”

so it seems something is needed here.

Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Alan Bateman]

On 23/07/2019 01:01, Brian Burkhalter wrote:

:
This version [1] adds writeBytes() and overrides write(byte[]) without changing 
its behavior. The documentation of write(byte[]) points the user to 
writeBytes() and write(byte[],int.int).

The overridden write(byte[]) declares that it throws IOE so it's source 
compatible but it behaves like the other PS.write methods in that 
checkError is used to check for an error rather than throwing IOE. This 
will need a bit more wording in the method description to make this 
clearer. The change to the class description is confusing as it suggests 
that the method throws IOE - maybe that change should be dropped to 
avoid trying to explain the oddity in two places.


-Alan.

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

[Dropping 2d-dev mailing list from the discussion as desktop is no longer 
affected.]

> On Jul 22, 2019, at 8:15 AM, Brian Burkhalter  
> wrote:
> 
>> On Jul 20, 2019, at 9:15 AM, Alan Bateman > > wrote:
>> 
>> On 18/07/2019 16:32, Brian Burkhalter wrote:
>>> Resuming this topic, what is the general view on the three possible paths:
>>> 
>>> 1. Override write(byte[]) at the risk of incompatibility.
>>> 2. Instead add writeBytes(byte[]) as in ByteArrayOutputStream.
>>> 3. Resolve as Won’t Fix.
>>> 
>>> For 2 or 3 the incorrect class level statement about overriding all methods 
>>> not to throw IOE would need to be dealt with.
>>> 
>> PrintStream dates from JDK 1.0 so I don't think it's feasible to add an 
>> override now that doesn't throw IOE.
> 
> Agreed.
> 
>> Tagir's suggestion to add a writeBytes(byte[]) seem fine, assuming there is 
>> a great need.
> 
> Not sure of the need but there is the precedent of BAIS.
> 
>> Alternatively you can add an override (that throws IOE) so that you have 
>> somewhere for an @apiNote that suggests using write(b,0,b.len) as 
>> "exception-less" write.
> 
> That’s a good idea.

This version [1] adds writeBytes() and overrides write(byte[]) without changing 
its behavior. The documentation of write(byte[]) points the user to 
writeBytes() and write(byte[],int.int).

Thanks,

Brian

[1] http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.01/

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> On Jul 20, 2019, at 9:15 AM, Alan Bateman  wrote:
> 
> On 18/07/2019 16:32, Brian Burkhalter wrote:
>> Resuming this topic, what is the general view on the three possible paths:
>> 
>> 1. Override write(byte[]) at the risk of incompatibility.
>> 2. Instead add writeBytes(byte[]) as in ByteArrayOutputStream.
>> 3. Resolve as Won’t Fix.
>> 
>> For 2 or 3 the incorrect class level statement about overriding all methods 
>> not to throw IOE would need to be dealt with.
>> 
> PrintStream dates from JDK 1.0 so I don't think it's feasible to add an 
> override now that doesn't throw IOE.

Agreed.

> Tagir's suggestion to add a writeBytes(byte[]) seem fine, assuming there is a 
> great need.

Not sure of the need but there is the precedent of BAIS.

> Alternatively you can add an override (that throws IOE) so that you have 
> somewhere for an @apiNote that suggests using write(b,0,b.len) as 
> "exception-less" write.

That’s a good idea.

Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Alan Bateman]

On 18/07/2019 16:32, Brian Burkhalter wrote:

Resuming this topic, what is the general view on the three possible paths:

1. Override write(byte[]) at the risk of incompatibility.
2. Instead add writeBytes(byte[]) as in ByteArrayOutputStream.
3. Resolve as Won’t Fix.

For 2 or 3 the incorrect class level statement about overriding all methods not 
to throw IOE would need to be dealt with.

PrintStream dates from JDK 1.0 so I don't think it's feasible to add an 
override now that doesn't throw IOE.


Tagir's suggestion to add a writeBytes(byte[]) seem fine, assuming there 
is a great need. Alternatively you can add an override (that throws IOE) 
so that you have somewhere for an @apiNote that suggests using 
write(b,0,b.len) as "exception-less" write.


-Alan

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

Resuming this topic, what is the general view on the three possible paths:

1. Override write(byte[]) at the risk of incompatibility.
2. Instead add writeBytes(byte[]) as in ByteArrayOutputStream.
3. Resolve as Won’t Fix.

For 2 or 3 the incorrect class level statement about overriding all methods not 
to throw IOE would need to be dealt with.

Thanks,

Brian

> On Jul 15, 2019, at 1:07 PM, Brian Burkhalter  
> wrote:
> 
> There is however this problematic statement in the PrintStream class doc that 
> neither of the two alternative versions of the fix addresses:
> 
> "Two other features are provided as well. Unlike other output streams, a 
> PrintStream never throws an IOException; instead, exceptional situations 
> merely set an internal flag that can be tested via the checkError method.”
> 
> In both cases this would need to be changed as it is incorrect.
> 
> Brian
> 
>> On Jul 15, 2019, at 10:14 AM, Brian Burkhalter > > wrote:
>> 
>> Here is an alternative version which adds a writeBytes(byte[]) method 
>> instead of overriding write(byte[]):
>> 
>> http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.00/ 
>>  
>> > >
>> 
>> This has the advantage of allowing new code to call writeBytes() without a 
>> try-catch block without introducing a compatibility issue for code which is 
>> already calling write(byte[]) in a try-catch block.

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

There is however this problematic statement in the PrintStream class doc that 
neither of the two alternative versions of the fix addresses:

"Two other features are provided as well. Unlike other output streams, a 
PrintStream never throws an IOException; instead, exceptional situations merely 
set an internal flag that can be tested via the checkError method.”

In both cases this would need to be changed as it is incorrect.

Brian

> On Jul 15, 2019, at 10:14 AM, Brian Burkhalter  
> wrote:
> 
> Here is an alternative version which adds a writeBytes(byte[]) method instead 
> of overriding write(byte[]):
> 
> http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.00/ 
>  
>  >
> 
> This has the advantage of allowing new code to call writeBytes() without a 
> try-catch block without introducing a compatibility issue for code which is 
> already calling write(byte[]) in a try-catch block.

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

Here is an alternative version which adds a writeBytes(byte[]) method instead 
of overriding write(byte[]):

http://cr.openjdk.java.net/~bpb/8187898/webrev-alt.00/ 


This has the advantage of allowing new code to call writeBytes() without a 
try-catch block without introducing a compatibility issue for code which is 
already calling write(byte[]) in a try-catch block.

Thanks,

Brian

> On Jul 15, 2019, at 9:00 AM, Brian Burkhalter  
> wrote:
> 
>> On Jul 13, 2019, at 12:00 PM, Tagir Valeev > > wrote:
>> 
>> And whilst great you are fixing up this code, we are but a small fraction of 
>> the world's code
>> that use java.io  and I wonder if this is worth the 
>> compatibility risk ?
>> 
>> Why not introducing a separate writeBytes method like it was done for 
>> ByteArrayOutputStream (JDK-8199713)?
> 
> Actually that seems like a better idea: accomplishes the objective while not 
> introducing a compatibility issue. Note: JDK 8199713 reference above is the 
> CSR of the actual issue [1].
> 
> Thanks,
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8180410 
> 

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Roger Riggs]

Looks fine,  Brian.

On 7/15/19 11:58 AM, Brian Burkhalter wrote:

So updated: http://cr.openjdk.java.net/~bpb/8187898/webrev.02/ 


Thanks,

Brian


On Jul 12, 2019, at 4:15 PM, Philip Race  wrote:

In that case just delete the comments and the commented out code ...

-phil.

On 7/12/19, 3:12 PM, Brian Burkhalter wrote:

I think I'd be more inclined to make the code at 970 like that at 1036.

Can’t do this exactly as drawImageBGR at 903 does not throw a PrinterException. 
Have to use RuntimeException.

Brian


On Jul 12, 2019, at 1:39 PM, Phil Race mailto:philip.r...@oracle.com>> wrote:

oh well ... in which I suppose this is the best you can do here.

-phil.

On 7/12/19 1:10 PM, Brian Burkhalter wrote:

(2) I don't know why at 1034 you changed from PrinterIOException to 
PrinterException.

There is no PrinterIOException constructor which accepts a String as its only 
parameter and there is no IOException to pass to a PrinterIOException 
constructor.

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

Hello,

> On Jul 13, 2019, at 12:00 PM, Tagir Valeev  wrote:
> 
> And whilst great you are fixing up this code, we are but a small fraction of 
> the world's code
> that use java.io  and I wonder if this is worth the 
> compatibility risk ?
> 
> Why not introducing a separate writeBytes method like it was done for 
> ByteArrayOutputStream (JDK-8199713)?

Actually that seems like a better idea: accomplishes the objective while not 
introducing a compatibility issue. Note: JDK 8199713 reference above is the CSR 
of the actual issue [1].

Thanks,

[1] https://bugs.openjdk.java.net/browse/JDK-8180410

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

So updated: http://cr.openjdk.java.net/~bpb/8187898/webrev.02/ 


Thanks,

Brian

> On Jul 12, 2019, at 4:15 PM, Philip Race  wrote:
> 
> In that case just delete the comments and the commented out code ... 
> 
> -phil.
> 
> On 7/12/19, 3:12 PM, Brian Burkhalter wrote:
>> 
>>> I think I'd be more inclined to make the code at 970 like that at 1036.
>> 
>> Can’t do this exactly as drawImageBGR at 903 does not throw a 
>> PrinterException. Have to use RuntimeException.
>> 
>> Brian
>> 
>>> On Jul 12, 2019, at 1:39 PM, Phil Race >> > wrote:
>>> 
>>> oh well ... in which I suppose this is the best you can do here.
>>> 
>>> -phil.
>>> 
>>> On 7/12/19 1:10 PM, Brian Burkhalter wrote:
 
> (2) I don't know why at 1034 you changed from PrinterIOException to 
> PrinterException.
 
 There is no PrinterIOException constructor which accepts a String as its 
 only parameter and there is no IOException to pass to a PrinterIOException 
 constructor.
>> 

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Tagir Valeev]

Hello!



> And whilst great you are fixing up this code, we are but a small fraction
> of the world's code
> that use java.io and I wonder if this is worth the compatibility risk ?
>

Why not introducing a separate writeBytes method like it was done for
ByteArrayOutputStream (JDK-8199713)?

With best regards,
Tagir Valeev


> -phil
>
>
>
> On 7/12/19 12:54 PM, Brian Burkhalter wrote:
> > Here is new webrev incorporating the two changes below.
> >
> > http://cr.openjdk.java.net/~bpb/8187898/webrev.01/
> >
> > Thanks,
> >
> > Brian
> >
> >> On Jul 12, 2019, at 12:01 PM, Brian Burkhalter <
> brian.burkhal...@oracle.com> wrote:
> >>
> >>> On Jul 12, 2019, at 11:17 AM, Roger Riggs  > wrote:
> >>>
> >>> Would it be appropriate to add @Override to the new method (and
> perhaps existing overridden methods).
> >> Yes, I think so.
> >>
> >>> Previously, calling FilterOutputStream.write(byte[]) would delegate to
> write(byte[], 0, length).
> >>> The proposed change duplicates the code and changes the ways that
> overridden classes might see the call.
> >>> What's the benefit of duplicating the code and calling out.write(buf)?
> >> Probably nothing. Probably better to call write(b,0,b.length) directly.
> >>
> >>> Not my call, but in PSPrinterJob.java (970-977) it might be cleaner to
> just delete the unused code and comment.
> >> I agree but I was waiting for a suggestion from a 2D reviewer.
>
>

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> I think I'd be more inclined to make the code at 970 like that at 1036.

Can’t do this exactly as drawImageBGR at 903 does not throw a PrinterException. 
Have to use RuntimeException.

Brian

> On Jul 12, 2019, at 1:39 PM, Phil Race  wrote:
> 
> oh well ... in which I suppose this is the best you can do here.
> 
> -phil.
> 
> On 7/12/19 1:10 PM, Brian Burkhalter wrote:
>> 
>>> (2) I don't know why at 1034 you changed from PrinterIOException to 
>>> PrinterException.
>> 
>> There is no PrinterIOException constructor which accepts a String as its 
>> only parameter and there is no IOException to pass to a PrinterIOException 
>> constructor.

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Phil Race]

oh well ... in which I suppose this is the best you can do here.

-phil.

On 7/12/19 1:10 PM, Brian Burkhalter wrote:


(2) I don't know why at 1034 you changed from PrinterIOException to 
PrinterException.


There is no PrinterIOException constructor which accepts a String as 
its only parameter and there is no IOException to pass to a 
PrinterIOException constructor.

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

> On Jul 12, 2019, at 1:02 PM, Phil Race  wrote:
> 
>> Not my call, but in PSPrinterJob.java (970-977) it might be cleaner to just 
>> delete the unused code and comment.
> 
> Dilemma - since at 1036 we are throwing an Exception and the comment at 
> 970-ish suggests
> the author was intending to do what s/he did at 1036 but never got around to 
> it.
> 
> I think I'd be more inclined to make the code at 970 like that at 1036.
> But
> (1) then the comment surely can go

OK

> (2) I don't know why at 1034 you changed from PrinterIOException to 
> PrinterException.

There is no PrinterIOException constructor which accepts a String as its only 
parameter and there is no IOException to pass to a PrinterIOException 
constructor.

> And whilst great you are fixing up this code, we are but a small fraction of 
> the world's code
> that use java.io  and I wonder if this is worth the 
> compatibility risk ?

I’m not sure either. I was leaving that decision to the CSR process.

Thanks,

Brian

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Phil Race]

Not my call, but in PSPrinterJob.java (970-977) it might be cleaner to just 
delete the unused code and comment.


Dilemma - since at 1036 we are throwing an Exception and the comment at 970-ish 
suggests
the author was intending to do what s/he did at 1036 but never got around to it.

I think I'd be more inclined to make the code at 970 like that at 1036.
But
(1) then the comment surely can go
(2) I don't know why at 1034 you changed from PrinterIOException to 
PrinterException.

And whilst great you are fixing up this code, we are but a small fraction of 
the world's code
that use java.io and I wonder if this is worth the compatibility risk ?

-phil



On 7/12/19 12:54 PM, Brian Burkhalter wrote:

Here is new webrev incorporating the two changes below.

http://cr.openjdk.java.net/~bpb/8187898/webrev.01/

Thanks,

Brian


On Jul 12, 2019, at 12:01 PM, Brian Burkhalter  
wrote:


On Jul 12, 2019, at 11:17 AM, Roger Riggs mailto:roger.ri...@oracle.com>> wrote:

Would it be appropriate to add @Override to the new method (and perhaps 
existing overridden methods).

Yes, I think so.


Previously, calling FilterOutputStream.write(byte[]) would delegate to 
write(byte[], 0, length).
The proposed change duplicates the code and changes the ways that overridden 
classes might see the call.
What's the benefit of duplicating the code and calling out.write(buf)?

Probably nothing. Probably better to call write(b,0,b.length) directly.


Not my call, but in PSPrinterJob.java (970-977) it might be cleaner to just 
delete the unused code and comment.

I agree but I was waiting for a suggestion from a 2D reviewer.

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Brian Burkhalter]

Here is new webrev incorporating the two changes below.

http://cr.openjdk.java.net/~bpb/8187898/webrev.01/

Thanks,

Brian

> On Jul 12, 2019, at 12:01 PM, Brian Burkhalter  
> wrote:
> 
>> On Jul 12, 2019, at 11:17 AM, Roger Riggs > > wrote:
>> 
>> Would it be appropriate to add @Override to the new method (and perhaps 
>> existing overridden methods).
> 
> Yes, I think so.
> 
>> Previously, calling FilterOutputStream.write(byte[]) would delegate to 
>> write(byte[], 0, length).
>> The proposed change duplicates the code and changes the ways that overridden 
>> classes might see the call.
>> What's the benefit of duplicating the code and calling out.write(buf)?
> 
> Probably nothing. Probably better to call write(b,0,b.length) directly.
> 
>> Not my call, but in PSPrinterJob.java (970-977) it might be cleaner to just 
>> delete the unused code and comment.
> 
> I agree but I was waiting for a suggestion from a 2D reviewer.