Memory leak in Observer pattern

[Violet_82]

HI guys, as I was playing with observers today I came across something interesting.
I have a program which keeps a list of users (Observers) and products (Observables) and the idea is that whenever the availability (that's a property of a product) changes the Observers are notified of this change. It all worked OK when there was 1 product with multiple users but now that I have multiple products things are falling apart and it seems I'm causing a memory leak somewhere - probably due to the fact thtat I'm not fully understanding the Observer pattern.
So when I try to print out the number of products per user (there should be only 3 product per user) the programs output the same product multiple times.
Let's have a look at the code and output:
Here is the product I'm observing:

//Product: this is the Observable
package observers;

import java.util.Observable;

public class Product extends Observable
{

    private boolean isAvailable = false;
    private String size = "";
    private String colour = "";
    private String name = "";

    public Product(boolean isAvailable, String size, String colour, String name)
    {
        this.name = name;
        this.isAvailable = isAvailable;
        this.size = size;
        this.colour = colour;
    }

    public String getName()
    {
        return name;
    }

    public void setName(String name)
    {
        this.name = name;
    }

    public boolean isAvailable()
    {
        return isAvailable;
    }
    public void setAvailable(boolean isAvailable)
    {
        this.isAvailable = isAvailable;
        //if(isAvailable()){
            setChanged();//set the flag to indicate that this observable has changed
            notifyObservers();//notify everyone
        //}
    }
    public String getSize()
    {
        return size;
    }
    public void setSize(String size)
    {
        this.size = size;
    }
    public String getColour()
    {
        return colour;
    }
    public void setColour(String colour)
    {
        this.colour = colour;
    }
    @Override
    public String toString(){
        return String.format("Product is: %s; Availability: %s; Size: %s; Colour: %s. ", this.getName(), this.isAvailable() ? "available" : "not available",  this.getSize(), this.getColour());
    }

}

Here are the observables, the users:

//User: these are the observers
package observers;

import java.util.ArrayList;
import java.util.List;
import java.util.Observable;
import java.util.Observer;

public class User implements Observer
{

    private String name = "";
    private List<Product> products = new ArrayList<Product>();

    public User(String name, List<Product> products)
    {
        this.name = name;
        this.products = products;
    }
    public void setName(String name)
    {
        this.name = name;
    }

    public String getName()
    {
        return name;
    }

    @Override
    public void update(Observable o, Object arg)
    {
        System.out.println("Username " + getName() + "'s update method called. " + o);

    }
    @Override
    public String toString(){
        return String.format("Username: %s", getName());
    }

}

And this is the class that test the whole thing, creating 3 products and 4 users:

package observers;

import java.util.ArrayList;
import java.util.List;
import java.util.Observer;

public class ObserverTest
{

    public static void main(String[] args)
    {
        List<Product> products = new ArrayList<>();
        List<Observer> users = new ArrayList<>();

        Product product = new Product(false, "small", "white", "jumper");
        Product product1 = new Product(false, "medium", "yellow", "shorts");
        Product product2 = new Product(false, "large", "red", "trousers");
        products.add(product);
        products.add(product1);
        products.add(product2);

        User user1 = new User("user1", products);
        User user2 = new User("user2", products);
        User user3 = new User("user3", products);
        User user4 = new User("user4", products);
        users.add(user1);
        users.add(user2);
        users.add(user3);
        users.add(user4);

        for(Observer user : users){
            for(Product currentProduct : products){
                currentProduct.addObserver(user);
                currentProduct.setAvailable(true);

            }
        }

        System.out.println("Num of observers: " + product.countObservers());
    }

}

I'm think the sticky point is this one:

for(Observer user : users){
            for(Product currentProduct : products){
                currentProduct.addObserver(user);
                currentProduct.setAvailable(true);

            }
        }

but I'm not 100% sure why it's tripping over this one because I add the user as the observer for each product and then when the setAvailable method is called all the observers are notified.
The output I get is this:

Username user1's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user1's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user1's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user2's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user1's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user2's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user1's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user2's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user1's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user3's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user2's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user1's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user3's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user2's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user1's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user3's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user2's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user1's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user4's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user3's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user2's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user1's update method called. Product is: jumper; Availability: available; Size: small; Colour: white. 
Username user4's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user3's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user2's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user1's update method called. Product is: shorts; Availability: available; Size: medium; Colour: yellow. 
Username user4's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user3's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user2's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Username user1's update method called. Product is: trousers; Availability: available; Size: large; Colour: red. 
Num of observers: 4

so 3 times for user4 which is what I expected but 6x for user3, 9x for user 2 and 12x for user1?!

[Violet_82]

Right, interesting. I fixed it, but alas, I don't understand why this fixes it. Basically rather than looping through users and then products and call setAvailable from there, so like this:

for(Observer user : users){
            for(Product currentProduct : products){
                currentProduct.addObserver(user);
                currentProduct.setAvailable(true);

            }
        }

I did this:

        for(Observer user : users){
            for(Product currentProduct : products){
                currentProduct.addObserver(user);
                //System.out.println(user + " " + currentProduct);
            }
        }
        for(Product currentProduct : products){
            currentProduct.setAvailable(true);
        }

and that works fine. Why can't I notify the observers inside the double for loop?

[John_191]

I don't believe that you ever really meant this:

        for(Observer user : users){
            for(Product currentProduct : products){
                currentProduct.addObserver(user);
                currentProduct.setAvailable(true);
            }
        }

You meant this:

        for(Product currentProduct : products){
            for(Observer user : users){
                currentProduct.addObserver(user);
            }
            currentProduct.setAvailable(true);
        }

The first way it adds the first user then notifies him for each product, then it adds the second user and notifies both him and the first again, then it adds the third and notifies EVERYONE again....

The second way it adds all the users to the first product, then notifies them, then adds all the users to the next product, then notifies them, etc.

[Violet_82]

The first way it adds the first user then notifies him for each product, then it adds the second user and notifies both him and the first again

Hang on why does it notify the first user again if the second iteration of the first loop is on the second user, meaning, why is everybody notified again and again?

[John_191]

How does it know you're "on the second user"? When you notifyObservers, all registered observers are notified. During the iteration for the second user, the first user has already been registered, so they are both notified.

[Violet_82]

OK got it, thanks for clarifying it.