2

Please consider the following code:

public abstract class Subject {
    private Collection<Observer> observerCollection = new HashSet<>();
    // ...
    protected void notifyObservers() {
        this.observerCollection.stream().filter(Objects::nonNull).forEach(o -> o.update(this));
    }
}

public interface Observer<T extends Subject> {
    void update(T subject);
}

I am getting the following compile-time warnings:

Observer is a raw type. References to generic type Observer should be parameterized

Type safety: The method update(Subject) belongs to the raw type Observer. References to generic type Observer should be parameterized

One comes at the call to update and for the life of me I can't figure out how to resolve it without using the warning suppressions. I've tried several ways to resolve the warning without luck. Any ideas on how this can be resolved?

Motivation

Consider the following client code:

public class IntegerContainer extends Subject {
    private int integer;
    
    public IntegerContainer(int integer) {
        this.integer = integer;
    }

    public int getInteger() {
        return this.integer;
    } // ...
}

public class IntegerObserver implements Observer<IntegerContainer> {
    private int cachedInteger;

    @Override
    public void update(IntegerContainer subject) {
        this.cachedInteger = subject.getInteger(); // avoid cast here.
    } // ...
}

The motivation for using generics in the Observer is to avoid a cast of the subject parameter so that the observer can retrieve the state of the subject.

Community
  • 1
  • 1
Raffi Khatchadourian
  • 3,042
  • 3
  • 31
  • 37
  • @LouisWasserman Yes, I've tried with the same result. I agree but just `Collection` gives me a `rawtype` and an `unchecked` warning. – Raffi Khatchadourian May 09 '17 at 17:52
  • You're using raw types. Don't do that. Also, don't name your type the same as well-known Java API types, especially not from the `java.lang....` or `java.util....` packages. – Lew Bloch May 09 '17 at 18:12
  • I'd like to take a step back: why do you need the Subject generic type? What kind of constraints do you want to enforce? I believe this `Observer.update` would allow pretty much every Subject and all their respective observers: `void update(Subject subject);` . – Tamas Rev May 10 '17 at 08:30
  • 1
    @TamasRev Great question! In the `update()` method of a class implementing `Observer`, I wanted to avoid a cast of the`Subject` parameter to a particular type in order to receive the state update. I'll augment the question to include a motivation. – Raffi Khatchadourian May 10 '17 at 13:42

2 Answers2

2

This doesn't have anything to do with streams; it just straight up won't work.

An Observer<? extends Subject> is more or less unusable, because you don't know what subtype of Subject it's an observer of. For all you know, observerCollection only contains an Observer<SomeSubtypeOfSubjectThatNobodyEvenHeardOf>. (See the PECS principle; Observer is a consumer.)

I don't think there's any type-safe way to do this cleanly, frankly, because you can't say in Subject that the attached observers all accept this subtype of Subject, because there's no way to refer to "this subtype of Subject." The closest hack I can think of is

abstract class Subject<T extends Subject<T>> {
  private Collection<Observer<? super T>> observers;
  protected void notifyObservers() {
    this.observerCollection.stream().filter(Objects::nonNull).forEach(o -> o.update((T) this)); // yes, this cast is unchecked
  }
}

class SubSubject extends Subject<SubSubject> {
  ...
}
Community
  • 1
  • 1
Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
  • Hm. Maybe I should change the question into how to avoid the warning. – Raffi Khatchadourian May 09 '17 at 18:03
  • Does the question make more sense now? Does it change your answer? – Raffi Khatchadourian May 09 '17 at 18:06
  • Not at all. There is no approach that will avoid the warning, not really, other than giving up on `Subject` entirely and having each of its subtypes do the observer stuff themselves. – Louis Wasserman May 09 '17 at 18:14
  • I don't know what the question looked like before the edit, but at the moment it's still showing the use of `Observer` (bad simple name) as a raw type. – Lew Bloch May 09 '17 at 18:16
  • @LouisWasserman Hm, but `this` is a `Subject` and `update()` takes a parameter of type `T` and `T` is defined to be some type extending `Subject`. I guess it boils down to whether a type "extends" itself, which think it ought to be considered as such. – Raffi Khatchadourian May 09 '17 at 18:16
  • @RaffiKhatchadourian, there is no way to convince the compiler that `this` is of type `T` other than an unsafe cast. – Louis Wasserman May 09 '17 at 18:17
  • @LouisWasserman That seems strange to me. It can't infer that `this` is in a method declared in the `Subject` class and that the upper bound of `T` is `Subject`? – Raffi Khatchadourian May 09 '17 at 18:24
  • 1
    @RaffiKhatchadourian, it can figure out both of those, but those premises aren't enough to prove that `this` is a `T`. `this` could be some _other_ subtype of `Subject`, besides `T`. – Louis Wasserman May 09 '17 at 18:29
0

I'd focus on the value being passed between the Subject and Observer. I.e. both classes have one type parameter and the related methods make sure that the types are compatible:

public interface Observer<T> {
    void update(T value); // no subject, but a value
}

public class Subject<T> {
      private Collection<Observer<? super T>> observers = new HashSet<>();

      protected void notifyObservers() {
        this.observers.stream().filter(Objects::nonNull).forEach(o -> o.update(this.getValue()));
      }

      public void addObserver(Observer<T> observer) { // adding the right kind of observer
          observers.add(observer);
      }

      abstract public T getValue(); // returning the value - this one is abstract
}

The key above is the abstract public T getValue(); method. Here is how you can write an IntegerContainer and and IntegerObserver :

public class IntegerContainer extends Subject<Integer> {

    private int value;

    public IntegerContainer(int value) {
        this.value = value;
    }

    @Override
    public Integer getValue() {
        return value; // this is the parameter of the update() call
        // you could even compute here something
        // you can pass entire objects too, if required
    }

}

public class IntegerObserver implements Observer<Integer> {
    private int cachedInteger;

    @Override
    public void update(Integer value) {
        this.cachedInteger = value; // no cast here
    } // ...
}

You can put them together like this:

IntegerContainer container = new IntegerContainer(3);
IntegerObserver observer = new IntegerObserver();
container.addObserver(observer);
container.notifyObservers();
Tamas Rev
  • 7,008
  • 5
  • 32
  • 49