harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lang Yang <yangl...@gmail.com>
Subject Re: [imageio] return a String array
Date Sun, 28 Mar 2010 22:53:49 GMT
Thanks, I will do so. Those variables are defined as "protected" in the
classes since we want those variables to be hidden. If we only return the
references to those variables, then users could access to them "publicly".

I also found that there few constructors don't clone mutable objects. like
this one:

package: javax.imageio.spi;

    public ImageReaderSpi(String vendorName, String version, String[] names,
String[] suffixes,
                             String[] MIMETypes, String pluginClassName,
                             Class[] inputTypes, String[] writerSpiNames,
                             boolean supportsStandardStreamMetadataFormat,
                             String nativeStreamMetadataFormatName,
                             String nativeStreamMetadataFormatClassName,
                             String[] extraStreamMetadataFormatNames,
                             String[] extraStreamMetadataFormatClassNames,
                             boolean supportsStandardImageMetadataFormat,
                             String nativeImageMetadataFormatName,
                             String nativeImageMetadataFormatClassName,
                             String[] extraImageMetadataFormatNames,
                             String[] extraImageMetadataFormatClassNames) {
        super(vendorName, version, names, suffixes, MIMETypes,
pluginClassName,
                supportsStandardStreamMetadataFormat,
nativeStreamMetadataFormatName,
                nativeStreamMetadataFormatClassName,
extraStreamMetadataFormatNames,
                extraStreamMetadataFormatClassNames,
supportsStandardImageMetadataFormat,
                nativeImageMetadataFormatName,
nativeImageMetadataFormatClassName,
                extraImageMetadataFormatNames,
extraImageMetadataFormatClassNames);

        if (inputTypes == null || inputTypes.length == 0) {
            throw new
NullPointerException(Messages.getString("imageio.5C"));
        }
        this.inputTypes = inputTypes;
        this.writerSpiNames = writerSpiNames;
    }

should we also change the code to:

        this.inputTypes = (inputTypes == null ? null : inputTypes.clone());
        this.writerSpiNames = (writerSpiNames ==  null ? null :
writerSpiNames.clone());

Is there an efficiency consideration for not cloning these objects?

Thanks,

Lang

On Sun, Mar 28, 2010 at 2:16 PM, Nathan Beyer <ndbeyer@apache.org> wrote:

> The javadoc for those methods isn't explicit about it being immutable,
> but I always assume in these cases, the data is a copy, because it's
> uncommon to leave these values "live" and that behavior is generally
> documented as such.
>
> So yet, it should probably be fixed.
>
> On Sun, Mar 28, 2010 at 11:42 AM, Yang Lang <yanglang@gmail.com> wrote:
> > Thanks Nathan, that’s really helpful.
> >
> > Both of them are in public classes.
> > javax.imageio.spi.ImageReaderWriterSpi.getFormatNames()
> > javax.imageio.spi.ImageWriterSpi.getImageReaderSpiNames()
> >
> > That’s why I was confused. In order to prevent the array be manipulated,
> > shouldn’t we always clone it in public APIs? There are few other methods
> in
> > javax package returns string[] without cloning. Can I assume this is a
> bug
> > and create a JIRA/patch for it?
> >
> > Thanks,
> >
> > Lang
> >
> > On Sun, Mar 28, 2010 at 1:47 AM, Nathan Beyer <ndbeyer@apache.org>
> wrote:
> >
> >> What's the context of each class? Is one class public (javax.*) and
> >> the other an internal class (org.apache.harmony.*)?
> >>
> >> This isn't something that's unique to String arrays, nor arrays;
> >> returning a copy of a field is a safety measure to ensure
> >> immutability, thread safety and other properties that an API may want
> >> to guarantee. Frequently, a public API will define certain classes as
> >> immutable, so the cloning of arrays is necessary, as the array's
> >> contents could be manipulated -- an array is not immutable.
> >>
> >> -Nathan
> >>
> >>
> >> On Sat, Mar 27, 2010 at 10:20 PM, Yang Lang <yanglang@gmail.com> wrote:
> >> > Hi guys,
> >> >
> >> > When I am reading through ImageIO package’s source code, I found out
> >> there
> >> > are two difference way to return a String[].
> >> >
> >> > For some methods, they call Arrays.clone() to clone a new string[] and
> >> > return the new one while some other methods returning the original
> >> String[]
> >> > directly.
> >> >
> >> > e.g.:
> >> > 1.
> >> > public String[] getFormatNames() {
> >> >    return names.clone();
> >> > }
> >> >
> >> > 2.
> >> > public String[] getImageReaderSpiNames() {
> >> >    return readerSpiNames;
> >> > }
> >> >
> >> > I am wondering what the difference between these two usages is. For
> what
> >> > kind of situations I need to clone a new array ?
> >> >
> >> > Thanks,
> >> >
> >> > Lang
> >> >
> >>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message