unchecked casts and clone

J

johnmmcparland

Hi all,

If I have a collection and wish to clone it, do I always need to
suppress the unchecked cast warning in Java 5? E.g.


import java.util.ArrayList;

public class Test
{
private ArrayList<String> names = new ArrayList<String>();

public void addName(String name)
{
names.add(name);
}

public String toString()
{
String s = "";
for (String name : names)
{
s += name + " ";
}
return s;
}

@SuppressWarnings("unchecked")
public Object clone()
{
Test other = new Test();
// Warning Type safety: Unchecked cast from Object to
ArrayList<String>
other.names = (ArrayList<String>)this.names.clone();
return other;
}

public static void main(String args[])
{
Test test1 = new Test();
test1.addName("John");
Test test2 = (Test)test1.clone();

// Check they're the same
System.out.print("Test1: ");
if (test1.toString().equals(test2.toString()))
{
System.out.println("Equal - good");
}
else
{
System.out.println("Unequal - bad");
}

// Check they're not shallow copied
test1.addName("Bob");
System.out.print("Test2: ");
if (test1.toString().equals(test2.toString()))
{
System.out.println("Equal - bad");
}
else
{
System.out.println("Unequal - good");
}
}
}

Look at the clone() method - I've indicated what line raises the
warning and temporarily suppressed it.
 
H

Hendrik Maryns

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

johnmmcparland schreef:
Hi all,

If I have a collection and wish to clone it, do I always need to
suppress the unchecked cast warning in Java 5? E.g.

Nope, use the copy constructors.
import java.util.ArrayList;

public class Test
{
private ArrayList<String> names = new ArrayList<String>();

Program to an interface, not to an implementation (google that!):

private List<String> names = new ArrayList<String>();

You’ll see that you no longer have the clone() method, but that is no
problem, see below.
@SuppressWarnings("unchecked")
public Object clone()
{
Test other = new Test();
// Warning Type safety: Unchecked cast from Object to
ArrayList<String>
other.names = (ArrayList<String>)this.names.clone();

Simply do

other.names = new ArrayList<String>(this.names);

and get rid of @SuppressWarnings.

H.
- --
Hendrik Maryns
http://tcl.sfs.uni-tuebingen.de/~hendrik/
==================
Ask smart questions, get good answers:
http://www.catb.org/~esr/faqs/smart-questions.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iEYEARECAAYFAkk/6D0ACgkQBGFP0CTku6MW7gCg0z3OR7Oo1apmdu3kqFHxSfdN
resAn1WhCxzrED46JSAglPkleOw9dVaG
=HdJ+
-----END PGP SIGNATURE-----
 
J

johnmmcparland

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

johnmmcparland schreef:



Nope, use the copy constructors.



Program to an interface, not to an implementation (google that!):

     private List<String> names = new ArrayList<String>();

You’ll see that you no longer have the clone() method, but that is no
problem, see below.


Simply do

        other.names = new ArrayList<String>(this.names);

and get rid of @SuppressWarnings.

H.
- --
Hendrik Marynshttp://tcl.sfs.uni-tuebingen.de/~hendrik/
==================
Ask smart questions, get good answers:http://www.catb.org/~esr/faqs/smart-questions.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with SUSE -http://enigmail.mozdev.org

iEYEARECAAYFAkk/6D0ACgkQBGFP0CTku6MW7gCg0z3OR7Oo1apmdu3kqFHxSfdN
resAn1WhCxzrED46JSAglPkleOw9dVaG
=HdJ+
-----END PGP SIGNATURE-----

excellent, cheers
 
L

Lew

johnmmcparland schreef:
Hendrik said:
Nope, use the copy constructors.

However, the OP's use of @SuppressedWarnings raises an important
point. Let's ignore Hendrik's excellent advice for a moment and go
with the OP's approach:

johnmmcparland schreef:
Well, except that should be a List as Hendrik said:
     private List<String> names = new ArrayList<String>();

johnmmcparland schreef:
The problem here is that you suppressed the warning for the whole
darned method! Just suppress it at the point where you do the
conversion from raw to generic, and document why it's safe to do the
conversion in a comment.

Don't document the danger, document what you did to make it safe!

// a clone of a List <String> so no ClassCastException
@SuppressWarnings( "unchecked" )
List <String> copy = (List <String>) this.names.clone();

other.names = copy;

Notice that we declare a local variable and suppress warnings just on
that local declaration. This will help prevent accidental blindness
to other unchecked casts in the method. By the time we set
other.names to the copy, type safety is back to being enforced at the
compiler level.
 
L

Lew

Lew said:
    // a clone of a List <String> so no ClassCastException
    @SuppressWarnings( "unchecked" )
    List <String> copy = (List <String>) this.names.clone();

For extra credit, why does this advice fail? It has nothing to do
with generics.

(Answer: element 'names' would have to be declared ArrayList, not
List.)
 
M

Mark Space

johnmmcparland said:
public class Test
{
public Object clone()
{
Test other = new Test();

This is a big problem right here. Test is non-final and you don't allow
for the case where there may be subclasses. I hope you don't do this in
the production code. Instead of calling "new Test()", please call
"super.clone()" instead.

Test other = (Test) super.clone();

Otherwise your code is going to be horribly broken.

If you make the class Test final, you can call a copy constructor on the
class itself, and use final fields too, which might be helpful.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top