Strange pointer issue when trying to strongly connect objects

In my program I have two classes, one called GlassPiece, and one called TrackerChip.

These two objects are always "strongly connected", that is, no two GlassPieces can share a TrackerChip, and no two TrackerChips can share a GlassPiece. Therefore in my setter methods, I need to take care to disconnect any old references hanging around, as so:

public class TrackerChip
{
    GlassPiece linkedGlassPiece;

    public void setGlassPiece(GlassPiece newGlassPiece)
    {
        GlassPiece oldGlassPiece = linkedGlassPiece;

        linkedGlassPiece = newGlassPiece;

        if(oldGlassPiece != null)
        {
            oldGlassPiece.setTrackerChip(null); //disconnect old GlassPiece
        }

        if(linkedGlassPiece != null && linkedGlassPiece.getTrackerChip() != this)
        {
            linkedGlassPiece.setTrackerChip(this); //update counterpart
        }
    }
}

and the method GlassPiece.setTrackerChip(TrackerChip) works exaxctly the same way.

The thing is, the above code doesn't actually work, and strange stuff happens when trying to manage li开发者_开发问答nking between several different GlassPieces and TrackerChips. However, if I replace the last part with:

    if(newGlassPiece != null && newGlassPiece.getTrackerChip() != this)
    {
        newGlassPiece.setTrackerChip(this);
    }

Then everything works properly. This seems very strange to me (all I did was replaced linkedGlassPiece, the instance variable, with newGlassPiece, the parameter). But early in the method I set the references equal to each other! Why does the first method not work?

P.S. I can confirm there is no infinite loop in the method.

---

As for why this isn't working, you're right, it won't hit an endless loop, but it's not going to do what you expect.

1. You enter setGlassPiece, linkedGlassPiece for this object is set to the value of newGlassPiece.
2. Then it calls setTrackerChip(null) on the oldGlassPiece.
3. The oldGlassPiece still has a reference to the original TrackerChip, so it calls setGlassPiece(null), which sets linkedGlassPiece to null that you just set on the TrackerChip, and calls setTrackerChip(null) on the NEW GlassPiece as well.

I honestly can't think of a way to get it to work the way you're going. You would have to add some additional parameters such that it would no longer be re-entrant. Namely, when you call setTrackerChip on the oldGlassPiece, it's not going to turn around and call the same TrackerChip back setting its reference to null. Perhaps just a boolean flag that would indicate that it should not null out the second level references.

Here's some code:

public class TrackerChip
{
    GlassPiece linkedGlassPiece;

    public void setGlassPiece(GlassPiece newGlassPiece)
    {
         setGlassPiece(newGlassPiece, true);
    }

    public void setGlassPiece(GlassPiece newGlassPiece, boolean reentrant)
    {
        GlassPiece oldGlassPiece = linkedGlassPiece;

        linkedGlassPiece = newGlassPiece;

        if(reentrant && oldGlassPiece != null)
        {
            oldGlassPiece.setTrackerChip(null, false); //disconnect old GlassPiece
        }

        if(linkedGlassPiece != null && linkedGlassPiece.getTrackerChip() != this)
        {
            linkedGlassPiece.setTrackerChip(this); //update counterpart
        }
    }
}

---

Instead of taking this approach, I would recommend just having a pair of static HashMaps that manage the relationships. That would probably be far simpler. There could be thread safety issues if your use case is not single threaded, but you'd just need to synchronize the method that sets it up. Maybe create a relationship management object as follows:

public class RelationshipMgr {
  HashMap<GlassPiece, TrackerChip> gpMap;
  HashMap<TrackerChip, GlassPiece> tcMap;

  public void setRelationship(GlassPiece gp, TrackerChip tc) {
    gpMap.put(gp, tc);
    tcMap.put(tc, gp);
  }
}

Actually, in the Google Guava library, there is even a class ready to use for this sort of thing called BiMap, check it out.

---

If there is always a one to one relationship, it might be worthwhile to consider merging those classes into one.

---

This should work I think:

public class TrackerChip {
    GlassPiece linkedGlassPiece;

    public void setGlassPiece(GlassPiece newGlassPiece) {
        if (linkedGlassPiece == newGlassPiece) {
            return;
        }
        if (linkedGlassPiece != null) {
            GlassPiece tmp = linkedGlassPiece;
            linkedGlassPiece = null;
            tmp.setTrackerChip(null);
        }
        if (newGlassPiece != null) {
            linkedGlassPiece = newGlassPiece;
            linkedGlassPiece.setTrackerChip(this);
        }
    }
}