CODEWITHSUNDEEP
javamultithreadingsynchronized
smsnheck
smsnheck

Reputation: 1593

Read and write into shared thread variables

first of all i am new to threads and shared variables. So please be kind with me ;-)

I'm having a class called Routing. This class recieves and handles messages. If a message is of type A the Routing-Object should pass it to the ASender Object which implements the Runnable Interface. If the message is of type B the Routing-Class should pass it to the BSender Object.

But the ASender and BSender Objects have common variables, that should be stored into the Routing-Object.

My idea now is to declare the variables as synchronized/volatile in the Routing-Object and the getter/setter also.

Is this the right way to synchronize the code? Or is something missing?

Edit: Added the basic code idea.

RoutingClass

public class Routing {

private synchronized Hashtable<Long, HashSet<String>> reverseLookup;
private ASender asender;
private BSender bsender;

public Routing() {
    //Constructor work to be done here.. 
    reverseLookup = new Hashtable<Long, HashSet<String>>();

}

public void notify(TopicEvent event) {

    if (event.getMessage() instanceof AMessage) {
        asender = new ASender(this, event.getMessage())

    } else if (event.getMessage() instanceof BMessage) {
        bsender = new BSender(this, event.getMessage())

    }
}

public synchronized void setReverseLookup(long l, Hashset<String> set) {
    reverseLookup.put(l, set);

}

public synchronized Hashtable<Long, Hashset<String>> getReverseLookup() {
    return reverseLookup;
}
}

ASender Class

public class ASender implements Runnable {

private Routing routing;
private RoutingMessage routingMessage;

public ASender(Routing r, RoutingMessage rm) {
    routing = r;
    routingMessage = rm;
    this.run();
}

public void run() {
    handleMessage();
}

private void handleMessage() {
    // do some stuff and extract data from the routing message object

    routing.setReverseLookup(somethingToSet)
}
}

Upvotes: 1

Views: 222

Answers (1)

Velth
Velth

Reputation: 1108

Some comments:

  1. Hashtable is a thread-safe implementation, you do not need another "synchronized" keyword see this and this for more information
  2. Avoid coupling, try to work with interfaces or pass the hashtable to your senders, see this for more information
  3. Depending on the amount of senders, you might want to use a ConcurrentHashMap, it greatly improves the performance, see ConcurrentHashMap and Hashtable in Java and Java theory and practice: Concurrent collections classes

This would conclude something like...:

public interface IRoutingHandling {

    void writeMessage(Long key, HashSet<String> value);

}

public class Routing implements IRoutingHandling {

    private final Hashtable<Long, HashSet<String>> reverseLookup;

    private ASender asender;
    private BSender bsender;

    public Routing() {
        //Constructor work to be done here.. 
        reverseLookup = new Hashtable<Long, HashSet<String>>();
    }

    public void notify(TopicEvent event) {
        if (event.getMessage() instanceof AMessage) {
            asender = new ASender(this, event.getMessage())

        } else if (event.getMessage() instanceof BMessage) {
            bsender = new BSender(this, event.getMessage())

        }
    }

    @Override
    public void writeMessage(Long key, HashSet<String> value) {
        reverseLookup.put(key, value);
    }

}

public class ASender implements Runnable {

    private IRoutingHandling _routingHandling;

    public ASender(IRoutingHandling r, RoutingMessage rm) {
        _routingHandling = r;
        routingMessage = rm;
        this.run();
    }

    public void run() {
        handleMessage();
    }

    private void handleMessage() {
        // do some stuff and extract data from the routing message object

        _routingHandling.writeMessage(somethingToSetAsKey, somethingToSetAsValue)
    }

}

Upvotes: 1

Related Questions