Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excessive usage of raw types #20

Closed
Marcono1234 opened this issue Dec 12, 2018 · 7 comments
Closed

Excessive usage of raw types #20

Marcono1234 opened this issue Dec 12, 2018 · 7 comments

Comments

@Marcono1234
Copy link
Contributor

Often generic types such as Tag do not have the type parameters specified (i.e. raw type), even in recent commits.

Raw types can become pretty dangerous, see this StackOverflow answer for a good explanation.

@Querz
Copy link
Owner

Querz commented Dec 13, 2018

I see your point and i'm working on it.

There is one Problem creating a ListTag of ListTags. Based on how you create ListTags using the constructor that takes the type class, it would be intuitive to do it this way:

ListTag<ListTag<?>> l = new ListTag<>(ListTag.class);

But obviously this doesn't compile because ListTag.class is not of type ListTag<?>.

And something like the following would be really ugly and having users of this library do this would be unreasonable:

ListTag<ListTag<?>> l = new ListTag<>((Class<ListTag<?>>)((Class<?>)ListTag.class));

Edit: Nevermind, i'm stupid. I can just modify the Constructor to accept a class of any type extending T

public ListTag(Class<? super T> typeClass)

@Marcono1234
Copy link
Contributor Author

public ListTag(Class<? super T> typeClass)

That should be ? extends T, otherwise it would allow stuff like new ListTag(Object.class).

@Querz
Copy link
Owner

Querz commented Dec 14, 2018

I am allowing the type of the class to be a superclass of T, which in turn must be a subclass of Tag<?> (Tag of any type), therefore the type of the class can't be a supertype of the ListTag's type.

If i restrict the type of the class to be a subclass of T instead, i have trouble creating a ListTag of ListTags:

ListTag<ListTag<?>> l = new ListTag<>(ListTag.class);
java: incompatible types: cannot infer type arguments for net.querz.nbt.ListTag<>
    reason: cannot infer type-variable(s) T
      (argument mismatch; java.lang.Class<net.querz.nbt.ListTag> cannot be converted to java.lang.Class<? extends net.querz.nbt.ListTag<?>>)

Also, this restricts the type of the class to be exactly the type of the ListTag, even though the definition is kinda verbose.

@Marcono1234
Copy link
Contributor Author

which in turn must be a subclass of Tag<?>
Where is this restriction?


I am not getting any compilation problems when changing the constructor parameter.

@Querz
Copy link
Owner

Querz commented Dec 15, 2018

Where is this restriction?

In the class definition:

public class ListTag<T extends Tag<?>> extends Tag<List<T>> implements Iterable<T> {

I am not getting any compilation problems when changing the constructor parameter.

Weird... I'm using Intellij IDEA 2018.2 to compile with JDK 1.8.0_181 and the unit tests won't compile with this error when i change the constructor parameter to Class<? extends T>:

[...]\NBT\src\test\java\net\querz\nbt\ListTagTest.java:242: error: incompatible types: cannot infer type arguments for ListTag<>
		ListTag<ListTag<?>> recursive = new ListTag<>(ListTag.class);
		                                           ^
    reason: cannot infer type-variable(s) T
      (argument mismatch; Class<ListTag> cannot be converted to Class<? extends ListTag<?>>)
  where T is a type-variable:
    T extends Tag<?> declared in class ListTag

@Querz
Copy link
Owner

Querz commented Dec 20, 2018

Removed all raw types.
The constructor of ListTag now takes a class of type ? super T while T extends Tag<?>, which restricts the class to be the exact same type as T but still allows a ListTag of type ListTag<?>:

ListTag<ListTag<?>> l = new ListTag<>(ListTag.class);

@Querz Querz closed this as completed Dec 20, 2018
@Marcono1234
Copy link
Contributor Author

Sorry that I have not said it before, but thanks for putting all this work into this project!


To the compiling problem again, it looks like it is working for me because I use Eclipse which uses its own compiler. There have been other cases where Eclipse compiled code just fine, but javac did not.

Here is the minimal test code, which should fail compiling for you:

public class ClassTest<T> {
    public ClassTest(Class<? extends T> classArg) { }
    
    public static void testNested() {
        // Compiles with Eclipse, but not with standard JDK
        ClassTest<ClassTest<?>> nested = new ClassTest<>(ClassTest.class);
    }
}

Hopefully I find the respective Eclipse bug report or get around to reporting it at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants