views:

257

answers:

7

Hey,

Let's say I have this type in my application:

public class A {
  public int id;
  public B b;

  public boolean equals(Object another) { return this.id == ((A)another).id; }
  public int hashCode() { return 31 * id; //nice prime number }
}

and a Set<A> structure. Now, I have an object of type A and want to do the following:

  • If my A is within the set, update its field b to match my object.
  • Else, add it to the set.

So checking if it is in there is easy enough (contains), and adding to the set is easy too. My question is this: how do I get a handle to update the object within? Interface Set doesn't have a get method, and the best I could think of was to remove the object in the set and add mine. another, even worse, alternative is to traverse the set with an iterator to try and locate the object.

I'll gladly take better suggestions... This includes the efficient use of other data structures.

Yuval =8-)

EDIT: Thank you all for answering... Unfortunately I can't 'accept' the best answers here, those that suggest using a Map, because changing the type of the collection radically for this purpose only would be a little extreme (this collection is already mapped through Hibernate...)

+4  A: 

Since a Set can only contain one instance of an object (as defined by its equals and hashCode methods), just remove it and then add it. If there was one already, that other one will be removed from the Set and replaced by the one you want.

I have code that does something similar - I am caching objects so that everywhere a particular object appears in a bunch of different places on the gui, it's always the same one. In that case, instead of using a Set I'm using a Map, and then I get an update, I retrieve it from the Map and update it in place rather than creating a new instance.

Paul Tomblin
That's not what 'add' does... If the set contains an equal object, 'add' return false. See http://java.sun.com/javase/6/docs/api/java/util/Set.html#add(E)
Yuval
I changed it to "remove then add". It's still a valid way of doing what you said you wanted, even if it's not what you meant.
Paul Tomblin
+7  A: 

You really want to use a Map<int,A>, not a Set<A>.

Then map the ID (even though it's also stored in A!) to the object. So storing new is this:

A a = ...;
Map<Integer,A> map = new HashMap<Integer,A>();
map.put( a.id, a );

Your complete update algorithm is:

public static void update( Map<Integer,A> map, A obj ) {
  A existing = map.get( obj.id );
  if ( existing == null )
     map.put( obj );
  else
     existing.b = obj.b;
}

However, it might be even simpler. I'm assuming you have more fields than that in A that what you gave. If this is not the case, just using a Map<Integer,B> is in fact what you want, then it collapses to nothing:

Map<Integer,B> map = new HashMap<Integer,B>();
// The insert-or-update is just this:
map.put( id, b );
Jason Cohen
Obviously there are more fields in A... In fact, the update I mean to make is in of the fields within one of A's fields! The key to A is not that simple either... =8-)
Yuval
A: 

It's a bit outside scope, but you forgot to re-implement hashCode(). When you override equals please override hashCode(), even in an example.

For example; contains() will very probably go wrong when you have a HashSet implementation of Set as the HashSet uses the hashCode of Object to locate the bucket (a number which has nothing to do with business logic), and only equals() the elements within that bucket.

public class A {
  public int id;
  public B b;
  public int hashCode() {return id;} // simple and efficient enough for small Sets 
  public boolean equals(Object another) { 
    if (object == null || ! (object instanceOf A) ) {
      return false;
    }
    return this.id == ((A)another).id; 
   }
}
public class Logic {
  /**
   * Replace the element in data with the same id as element, or add element
   * to data when the id of element is not yet used by any A in data. 
   */
  public void update(Set<A> data, A element) {
    data.remove(element); // Safe even if the element is not in the Set
    data.add(element); 
  }
}

EDIT Yuvalindicated correctly that Set.add does not overwrite an existing element, but only adds if the element is not yet in the collection (with "is" implemented by equals)

extraneon
Edited the question to add hashCode()... so right.
Yuval
Your solution is similar to that of Paul Tomblin, and sadly is similarly wrong... check out the javadoc for Set#add, or just try it out.
Yuval
A: 

What about Map<A,A> I know it's redundant, but I believe it will get you the behavior you'd like. Really I'd love to see Set have a get(Object o) method on it.

A: 

You might want to generate a decorator called ASet and use an internal Map as the backing data structure

class ASet {
 private Map<Integer, A> map;
 public ASet() {
  map = new HashMap<Integer, A>();
 }

 public A updateOrAdd(Integer id, int delta) {
   A a = map.get(a);
   if(a == null) {
    a = new A(id);
    map.put(id,a);
   }
   a.setX(a.getX() + delta);
 }
}

You can also take a look at the Trove API. While that is better for performance and for accounting that you are working with primitive variables, it exposes this feature very nicely (e.g. map.adjustOrPutValue(key, initialValue, deltaValue).

Josh
+1  A: 

I don't think you can make it any easier than using remove/add if you are using a Set.

    set.remove(a);
    set.add(a);

If a matching A was found it will be removed and then you add the new one, you don't even need the if (set.contains(A)) conditional.

If you have an object with an ID and an updated field and you don't really care about any other aspects of that object, just throw it out and replace it.

If you need to do anything else to the A that matches that ID then you'll have to iterate through the Set to find it or use a different Container (like the Map as Jason suggested).

18Rabbit
+1  A: 

No one has mentioned this yet, but basing hashcode or equals on a mutable property is one of those really, really big things that you shouldn't do. Don't muck about with object identity after you leave the constructor - doing so greatly increases your chances of having really difficult-to-figure out bugs down the road. Even if you don't get hit with bugs, the accounting work to make sure that you always properly update any and all data structures that relies on equals and hashcode being consistent will far outweigh any perceived benefits of being able to just change the id of the object as you run.

Instead, I strongly recommend that you pass id in via the constructor, and if you need to change it, create a new instance of A. This will force users of your object (including yourself) to properly interact with the collection classes (and many others) that rely on immutable behavior in equals and hashcode.

Kevin Day