0

I have a two part question.

I have: private static ConcurrentHashMap<Integer, Servers> servers= null; which I later populate. In method getAllServers, I'm doing this:

public static synchronized ConcurrentHashMap<Integer, Server> getAllServers() {
    ConcurrentHashMap<Integer, Server> toReturn = new ConcurrentHashMap<>();
    for(Integer i = 0; i < servers.size(); i ++ ) {
        Server s = servers.get(i);
        if(s.isAlive()) {
            s.incrementConns();
            toReturn.put(i, s);
        }
    }
    return toReturn;
}

Q: Will modifications to s be reflected in servers?
Q: Will toReturn reference the same Server objects as in servers? If so, would any class that creates and modifies getAllServers's returned map be modifiying the same objects as servers?

Update: @chrylis: Which Iterator are you referring to? Can you please explain why the for loop is a bad idea a bit more? I do use this as well, in other areas: 

Iterator itr = servers.entrySet().iterator();
            Map.Entry pair;
            while(itr.hasNext()) {
                pair = (Map.Entry)itr.next();
                Server s= (Server) pair.getValue();
                ...

But I don't see anything wrong since I know servers will contain Servers with Integer ids ranging from 0 onward. When I iterate over them in the for loop, the order is not a concern for me.

foamroll
  • 752
  • 7
  • 23
  • 1
    Read [this](http://docs.oracle.com/javase/tutorial/java/javaOO/usingobject.html) to start. `ConcurrentHashMap` isn't really important. You need to understand object references. – Sotirios Delimanolis Aug 28 '13 at 15:55
  • 1
    For starters, your loop is quite brittle; there's no guarantee that your keys are a sequential set of numbers, and if there were, a `List` would make more sense. Is there reason you're not iterating over `servers.entrySet()`? Also, you are talking about "value objects", but `s.incrementConns()` sounds like a mutator. – chrylis -cautiouslyoptimistic- Aug 28 '13 at 15:55
  • 1
    Yes, and Yes, and Yes – Cruncher Aug 28 '13 at 15:56
  • @chrylis: the key used for Server objects is an Integer parameter in the class, which is assigned to the objects at their construction in a static ServerCreator class. Sequence doesn't matter, as long as the iteration goes through all the values of the map, which are Server objects. Yes, `incrementConns()` is a mutator that modifies another parameter of the object - is using a mutator in the same loop a bad idea? – foamroll Aug 28 '13 at 16:08
  • The iterator *won't* go through all of the values unless they are always, always, exactly 0 through the length of the map minus one. – chrylis -cautiouslyoptimistic- Aug 28 '13 at 17:50

2 Answers2

3

Yes and yes. It may not even be possible to copy Server objects, as far as this code is concerned. This has nothing to do with ConcurrentHashMap, and everything to do with how references work in Java.

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
2
  1. servers only hold references to the Server objects: whatever modification you make to the underlying objects will be reflected in servers too.
  2. Java passes references by value, so yes.

Note that if your code is not properly synchronized, you might see stale and/or inconsistent Server objects in your code, but that is a different issue.

Community
  • 1
  • 1
assylias
  • 321,522
  • 82
  • 660
  • 783