CODEWITHSUNDEEP
javaclassgenericsmethodsreturn
stonar96
stonar96

Reputation: 1461

Java generics method

First of all, I have not much experience with java generics.

I hava a method kind of that:

public static Set<Object> getObjects(Class<? extends Object> clazz) {
    if (clazz == null) {
        clazz = Object.class;
    }

    Set<Object> result = new HashSet<Object>();
    Object object;

    for (/*a loop*/) {
        object = /*get an object*/;

        if (clazz.isInstance(object)) {
            result.add(object);
        }
    }

    return result;
}

And I thought it would be better to use generics, to get the right return type. So I changed the method to this:

public static <T> Set<T> getObjects(Class<T> clazz) {
    if (clazz == null) {
        //what should I do instead? Throw exception?
    }

    Set<T> result = new HashSet<T>();
    Object object;

    for (/*a loop*/) {
        object = /*get an object*/;

        if (clazz.isInstance(object)) {//or should I use object instanceof T? If yes, I don't event use clazz. Would it be bad practice?
            result.add((T) object);
        }
    }

    return result;
}

My questions: See comments in second code. Should I use generics here? Do I use it right? How would you do it?

Upvotes: 0

Views: 100

Answers (1)

sisyphus
sisyphus

Reputation: 6392

First, yes, you should probably throw an IllegalArgumentException or NullPointerException if null is passed as the method argument.

Also, this line

result.add((T) object);

should probably use clazz to do the cast. Because the T is erased you don't get any safety from the casting and BadThings(tm) can happen. So, it should really look like

result.add(clazz.cast(object));

Also, if you're using Java8 then you might like to consider using the streams API. Assuming there's some method you can call which returns a collection of 'all' the objects, then your method would look like

 public static <T> Set<T> getObjects(Class<T> clazz) {
      if (clazz == null) {
          throw new IllegalArgumentException();
      }
      return getAllTheObjects().stream()
                               .filter(clazz::isInstance)
                               .map(clazz::cast)
                               .collect(Collectors.toSet());
 }

Upvotes: 4

Related Questions