patternjavaMinor
Removing an object which is wrapped in another object from a collection
Viewed 0 times
fromremovingcollectionanotherwrappedwhichobject
Problem
I have
The trouble comes when I want to remove a
Within the
I would like to avoid burdening the users of this class with requiring them to choose a unique token to identify each instance of
Does anyone have a good way of cleaning this up?
Clips that can be added to or removed from any number of Lines. When I add the Clip, I put it into a ClipHolder so I can keep some bounds information with it which is specific to the Line / Clip relationship.The trouble comes when I want to remove a
Clip from the Line. I need to remove the ClipHolder in the associatedClips Set which holds the Clip, but I cannot just call associateClips.remove(Clip) (obviously as the set is holding ClipHolders, not Clips) so I am stuck iterating through the set, and searching one by one for it (see the removeClip method below.)Within the
Line class the ClipHolder class is simply an implementation detail that the user should not have to worry about. They should only have to worry about their Clips which are a separate class.I would like to avoid burdening the users of this class with requiring them to choose a unique token to identify each instance of
Clip. I think that having a reference to the Clip should be enough. (Is this reasonable?)Does anyone have a good way of cleaning this up?
class Line {
.....
private class ClipHolder{
Clip c;
Bounds bounds;
}
private TreeSet associatedClips = new TreeSet();
public void addClip(Clip c){
ClipHolder holder = new ClipHolder();
holder.c = c;
calculateBounds(holder);
associatedClips.add(holder);
}
public void removeClip(Clip c){
for (ClipHolder ih: associatedClips ) {
if(ih.c == c){
associatedClips.remove(ih);
break;
}
}
}
}Solution
All you need is a (hash) map for mapping a
Line.java:
Clip to its ClipHolder. That way you can remove Clips in constant time. I have added a couple of comments directly in the code snippet:Line.java:
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
class Line {
private static class Bounds {
//...
}
// Clip is immutable. It is a good idea to override 'hashCode' and 'equals'
// in order to make sure that we can safely use 'Clip's as keys in a hash
// map.
private static class Clip {
private final String name;
Clip(String name) {
this.name = name;
}
public String getName() {
return name;
}
@Override
public int hashCode() {
return name.hashCode();
}
@Override
public boolean equals(Object o) {
if (!(o instanceof Clip)) {
return false;
}
return ((Clip) o).getName().equals(this.getName());
}
@Override
public String toString() {
return "[Clip: " + name + "]";
}
}
// Keyword 'static' removes an implicit reference to Line from each
// 'ClipHolder', thus saving some space. Declare your ClipHolder a
// 'Comparable'.
private static class ClipHolder implements Comparable {
private final Clip clip;
private final Bounds bounds;
ClipHolder(Clip clip, Bounds bounds) {
this.clip = clip;
this.bounds = bounds;
}
@Override
public int compareTo(ClipHolder o) {
return this.getClip().getName().compareTo(o.getClip().getName());
}
public Clip getClip() {
return clip;
}
public Bounds getBounds() {
return bounds;
}
}
// 1: Program to interface, not implementation.
// 2: Since Java 7, you can use diamond inference "<>".
private final Set associatedClips = new TreeSet<>();
private final Map map = new HashMap<>();
public void addClip(Clip clip) {
Bounds bounds = null; // Compute your bounds.
ClipHolder holder = new ClipHolder(clip, bounds);
associatedClips.add(holder);
// Map a clip to its holder.
map.put(clip, holder);
}
public void removeClip(Clip c) {
associatedClips.remove(map.get(c));
}
@Override
public String toString() {
StringBuilder sb = new StringBuilder("[Line: ");
int i = 0;
for (ClipHolder ch : associatedClips) {
sb.append(ch.getClip().toString());
if (i < associatedClips.size() - 1) {
sb.append(", ");
++i;
}
}
return sb.append("]").toString();
}
public static void main(String[] args) {
Line line = new Line();
line.addClip(new Clip("A"));
line.addClip(new Clip("B"));
line.addClip(new Clip("C"));
line.addClip(new Clip("D"));
System.out.println(line);
line.removeClip(new Clip("B"));
System.out.println(line);
}
}Code Snippets
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
class Line {
private static class Bounds {
//...
}
// Clip is immutable. It is a good idea to override 'hashCode' and 'equals'
// in order to make sure that we can safely use 'Clip's as keys in a hash
// map.
private static class Clip {
private final String name;
Clip(String name) {
this.name = name;
}
public String getName() {
return name;
}
@Override
public int hashCode() {
return name.hashCode();
}
@Override
public boolean equals(Object o) {
if (!(o instanceof Clip)) {
return false;
}
return ((Clip) o).getName().equals(this.getName());
}
@Override
public String toString() {
return "[Clip: " + name + "]";
}
}
// Keyword 'static' removes an implicit reference to Line from each
// 'ClipHolder', thus saving some space. Declare your ClipHolder a
// 'Comparable'.
private static class ClipHolder implements Comparable<ClipHolder> {
private final Clip clip;
private final Bounds bounds;
ClipHolder(Clip clip, Bounds bounds) {
this.clip = clip;
this.bounds = bounds;
}
@Override
public int compareTo(ClipHolder o) {
return this.getClip().getName().compareTo(o.getClip().getName());
}
public Clip getClip() {
return clip;
}
public Bounds getBounds() {
return bounds;
}
}
// 1: Program to interface, not implementation.
// 2: Since Java 7, you can use diamond inference "<>".
private final Set<ClipHolder> associatedClips = new TreeSet<>();
private final Map<Clip, ClipHolder> map = new HashMap<>();
public void addClip(Clip clip) {
Bounds bounds = null; // Compute your bounds.
ClipHolder holder = new ClipHolder(clip, bounds);
associatedClips.add(holder);
// Map a clip to its holder.
map.put(clip, holder);
}
public void removeClip(Clip c) {
associatedClips.remove(map.get(c));
}
@Override
public String toString() {
StringBuilder sb = new StringBuilder("[Line: ");
int i = 0;
for (ClipHolder ch : associatedClips) {
sb.append(ch.getClip().toString());
if (i < associatedClips.size() - 1) {
sb.append(", ");
++i;
}
}
return sb.append("]").toString();
}
public static void main(String[] args) {
Line line = new Line();
line.addClip(new Clip("A"));
line.addClip(new Clip("B"));
line.addClip(new Clip("C"));
line.addClip(new Clip("D"));
System.out.println(line);
line.removeClip(new Clip("B"));
Context
StackExchange Code Review Q#99322, answer score: 3
Revisions (0)
No revisions yet.