1

I'm trying to clean up some legacy code and have some unchecked cast warnings I'm struggeling to get rid of.

I've extracted the code giving the warnings into a compilable program below. Note that I removed much of the code to make it smaller so all of it might not make complete sense. It compiles, but running it wont do anything. 

import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.List; import java.util.zip.ZipEntry; import java.util.zip.ZipException; import java.util.zip.ZipInputStream; public class GenericsTest { public static void main(String[] args) throws IOException { Reader reader = new Reader(); List<String> stringEntries = reader.readAll(StringEntry.class); } public static class Reader { public <T> ZipInputStream getInputStream(String fileName) throws ZipException, FileNotFoundException { return new ZipInputStream(new FileInputStream(fileName));//_file.getInputStream(_paths.get(fileName)); } public <T, TEntry extends Entry<T>> List<T> readAll(Class<TEntry> type) throws IOException { List<T> list = new ArrayList<T>(); List<TEntry> entries = createEntries(type); for (TEntry entry : entries) { list.add(read(entry)); } return list; } public <T> T read(Entry<T> entry) throws IOException { ZipInputStream is = null; try { //is = _archive.getInputStream(entry.getName()); return entry.read(is); } finally { if (is != null) { is.close(); } } } public <TEntry extends Entry> List<TEntry> createEntries(Class<TEntry> type) throws ZipException { List<TEntry> entries = new ArrayList<TEntry>(); List<String> paths = new ArrayList<String>();//getPaths(type); for (String path : paths) { entries.add(createEntry(type, path)); } return entries; } public <TEntry extends Entry> TEntry createEntry(Class<TEntry> type, String folder) { if (StringEntry.class.equals(type)) { return (TEntry) new StringEntry(folder); } else if (IntegerEntry.class.equals(type)) { return (TEntry) new IntegerEntry(folder); } throw new IllegalArgumentException("Unknown type: " + type); } } public static abstract class Entry<T> extends ZipEntry { private T _data; public Entry(T data, String folder, String name) { super(folder + "/" + name); _data = data; } protected abstract T read(InputStream is) throws IOException; }; public static class StringEntry extends Entry<String> { public StringEntry(String folder) { super("Hallo world!", folder, "StringEntry"); } @Override protected String read(InputStream is) throws IOException { throw new UnsupportedOperationException("Not supported yet."); } }; public static class IntegerEntry extends Entry<Integer> { public IntegerEntry(String folder) { super(42, folder, "IntegerEntry"); } @Override protected Integer read(InputStream is) throws IOException { throw new UnsupportedOperationException("Not supported yet."); } }; }

Compiling the above code gives the following warnings

GenericsTest.java:57: warning: [unchecked] unchecked cast found: GenericsTest.StringEntry required: TEntry return (TEntry) new StringEntry(folder); GenericsTest.java:59: warning: [unchecked] unchecked cast found: GenericsTest.IntegerEntry required: TEntry return (TEntry) new IntegerEntry(folder); 2 warnings

Changing 

public <TEntry extends Entry> TEntry createEntry...

to 

public <TEntry extends Entry<T>> TEntry createEntry...

gives a compiler error (cannot find symbol: class T).

I don't want to change the code too much since it is working fine, so how can I fix (not hide) the warnings with the least code changes?

Improve this question

3 Answers 3

Reset to default
2

You're already passing the Class object into the method, just use Class.cast():

public <TEntry extends Entry<?>> TEntry createEntry(Class<TEntry> type, String folder) { if (StringEntry.class.equals(type)) { return type.cast(new StringEntry(folder)); } else if (IntegerEntry.class.equals(type)) { return type.cast(new IntegerEntry(folder)); } throw new IllegalArgumentException("Unknown type: " + type); }

The above change makes your code compile without warnings for me.

Also, the <T> in the signature of getInputStream() is probably not necessary.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

I did not know about Class.cast()! Learning something new every day :) Thanks!
@ughzan Class.cast() is more or less the main way on how to get rid of unchecked cast warnings. Its main limitation is that you can't have a class object for Entry<String> (as opposed to a StringEntry class that extends it), so you can't cast to it.
2

If you want to use more than one generic parameter you will need to specify it in the parameter list. Can you try this?

public <T, TEntry extends Entry<T>> TEntry createEntry(...

UPDATE

I had some time to play with this and put those nice class objects in use:

 public <T, TEntry extends Entry<T>> List<TEntry> createEntries(Class<TEntry> type) throws ZipException { List<TEntry> entries = new ArrayList<TEntry>(); List<String> paths = new ArrayList<String>();//getPaths(type); for (String path : paths) { entries.add(createEntry(type, path)); } return entries; } public <T, TEntry extends Entry<T>> TEntry createEntry(Class<TEntry> type, String folder) { if (StringEntry.class.equals(type)) { return type.cast(new StringEntry(folder)); } else if (IntegerEntry.class.equals(type)) { return type.cast(new IntegerEntry(folder)); } throw new IllegalArgumentException("Unknown type: " + type); } }
Improve this answer

3 Comments

Did you actually try this to see if it makes the compiler warning go away?
The original answer: no; the updated answer compiles for me (JDK 1.6)
The T is redundant in the update, but that's just a niggle.
0

IMHO for legacy code, to make it generic friendly can be quite an experience, specially if you have a large base of code... Since probably this code has been deployed, would you not consider an iterative approach?

By iterative approach I mean... Get all your warning and eliminate them with @SuppressWarning, and with a TODO annotation then, as the time goes by, every time you need to do a change in your code, if you stumble upon a TODO, just change it then...

I am just saying in case that you are just migrating to Java 5 and have thousands of warnings..

Improve this answer

4 Comments

@SuppressWarnings should be a last resort option for well-encapsulated hacks that you know are correct, preferably have tests in place to make sure they remain correct, and when you know you can't get rid of the warnings in a clean way, or at least not without more bother than it's worth. It's not something you're supposed to sprinkle around your codebase liberally to make the compiler stop making you feel guilty.
I completely agree with you... I was just talking in case you are going through a legacy project migrating to Java 5, sometimes is just not affordable to go through the correct generics... That would be for me the only exception where @SuppressWarning would be acceptable
In that case, I'd just leave the warnings.
Then, you wouldn't know if in your new code, the one that you are going to write on the top of the legacy code you are having more warnings... You won't care about the warnings anymore

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.