Re: [jira] Updated: (HARMONY-156) InputStreamReader.getEncoding() and OutputStreamWriter.getEncoding() should return a historical charset name.

["Dmitry M. Kononov" <dm...@gmail.com>]

Paulex,

I have some thoughts about the patch. It looks good but the following two
things:

1) java/io/InputStreamReader.
getHistoricalName(String name) should not be public I think.

2) java/io/OutputStreamWriter.getEncoding().
You probably meant

if (!isOpen()) {
    return null;
}

instead of

if (encoder == null) {
    return null;
}

since this method returns null if the stream has been closed.

Thanks.

On 3/8/06, Paulex Yang (JIRA) <ji...@apache.org> wrote:
>
>     [ http://issues.apache.org/jira/browse/HARMONY-156?page=all ]
>
> Paulex Yang updated HARMONY-156:
> --------------------------------
>
>    Attachment: patch_156.txt
>                patch_156_test.txt
>
> As no one has better idea than my prior proposal, i.e., save the mapping
> between historical name and canonical name, I created the patch for this
> solution. The affected package is luni/java.io.
>
> --
> Dmitry M. Kononov
> Intel Managed Runtime Division

Re: [jira] Updated: (HARMONY-156) InputStreamReader.getEncoding() and OutputStreamWriter.getEncoding() should return a historical charset name.

["Dmitry M. Kononov" <dm...@gmail.com>]

On 3/10/06, Paulex Yang <pa...@gmail.com> wrote:
>
> > 1) java/io/InputStreamReader.
> > getHistoricalName(String name) should not be public I think.
> >
> the getHistoricalName(String) is not method of InputStreamReader, but a
> method of package private internal class
> InputStreamReader.HistoricalNamesUtil, so it should be OK to be public.


Yes, you are right.


> > 2) java/io/OutputStreamWriter.getEncoding().
> > You probably meant
> >
> > if (!isOpen()) {
> >     return null;
> > }
> >
> > instead of
> >
> > if (encoder == null) {
> >     return null;
> > }
> >
> There is no corresponding isOpen() method in OutputStreamWriter, like
> InputStreamReader, it's weird but true:), and because the encoder is set
> to null at the close() method, so that the encoder==null is fine. And of
> course again, isOpen() is more self-descriptive than encoder==null, so
> this method should be added into OutputStreamWriter.


You are right again. Sorry for my issues.
The fix looks good to me now. :)

--
> Dmitry M. Kononov
> Intel Managed Runtime Division

Re: [jira] Updated: (HARMONY-156) InputStreamReader.getEncoding() and OutputStreamWriter.getEncoding() should return a historical charset name.

[Paulex Yang <pa...@gmail.com>]

Kononov,

Thank you to point out these. My comments below.

Dmitry M. Kononov wrote:
> Paulex,
>
> I have some thoughts about the patch. It looks good but the following two
> things:
>
> 1) java/io/InputStreamReader.
> getHistoricalName(String name) should not be public I think.
>   
the getHistoricalName(String) is not method of InputStreamReader, but a 
method of package private internal class 
InputStreamReader.HistoricalNamesUtil, so it should be OK to be public. 
And of course, it is more precise to make it package private.
> 2) java/io/OutputStreamWriter.getEncoding().
> You probably meant
>
> if (!isOpen()) {
>     return null;
> }
>
> instead of
>
> if (encoder == null) {
>     return null;
> }
>   
There is no corresponding isOpen() method in OutputStreamWriter, like 
InputStreamReader, it's weird but true:), and because the encoder is set 
to null at the close() method, so that the encoder==null is fine. And of 
course again, isOpen() is more self-descriptive than encoder==null, so 
this method should be added into OutputStreamWriter.
> since this method returns null if the stream has been closed.
>
> Thanks.
>
> On 3/8/06, Paulex Yang (JIRA) <ji...@apache.org> wrote:
>   
>>     [ http://issues.apache.org/jira/browse/HARMONY-156?page=all ]
>>
>> Paulex Yang updated HARMONY-156:
>> --------------------------------
>>
>>    Attachment: patch_156.txt
>>                patch_156_test.txt
>>
>> As no one has better idea than my prior proposal, i.e., save the mapping
>> between historical name and canonical name, I created the patch for this
>> solution. The affected package is luni/java.io.
>>
>> --
>> Dmitry M. Kononov
>> Intel Managed Runtime Division
>>     
>
>   


-- 
Paulex Yang
China Software Development Lab
IBM