Avoiding duplicate entries in an array

Question

I am writing a method that adds Vertex objects to an array. I need to check if the vertex I am adding already exists in the array. I am not sure where I am going wrong. Here is my method:

public void addVertex(Vertex v) {
    if (activeVertices >= maxVertices) {
        System.out.println("Graph full");
        return;
    }
    for(int i=1; i<vertices.length; i++) {
        if(vertices[i] != vertices[i-1]){
            vertices[activeVertices] = v; // add vertex to list of vertices
            v.graphIndex = activeVertices; // record index of vertex in graph
            activeVertices++;  // increment vertex count
        }
    }
}

Vertex class:

public class Vertex {
    public String name;
    public int graphIndex; // index of adjacency matrix position of node in graph

    public Vertex (String s) {
        name = s;
        graphIndex = -1; // invalid position by default 
    }

    public String toString() {
        return name;
    }

}

The class that contains the addVertex() method:

public class Graph {
    private int maxVertices;
    private Vertex[] vertices; // array of nodes
    private int[][] edges; // adjacency matrix
    int activeVertices;


    public Graph(int maxSize) {
        maxVertices = maxSize;
        vertices = new Vertex[maxVertices];
        activeVertices = 0;
    }


    public void addVertex(Vertex v) {
        if (activeVertices >= maxVertices) {
            System.out.println("Graph full");
            return;
        }
        for(int i=1; i<vertices.length; i++) {
            if(vertices[i] != vertices[i-1]){
                vertices[activeVertices] = v; // add vertex to list of vertices
                v.graphIndex = activeVertices; // record index of vertex in graph
                activeVertices++;  // increment vertex count
            }
        }
    }
    public void addEdge(Vertex v1, Vertex v2, int w) {
        edges[v1.graphIndex][v2.graphIndex] = w;
        edges[v2.graphIndex][v1.graphIndex] = w;
    }

    public Graph minimumSpanningTree() {
        Graph mst = new Graph(maxVertices); // create new graph
        int[] set = new int[activeVertices];
        for (int i=0; i<activeVertices; i++) { // copy nodes to graph
            mst.addVertex(vertices[i]);
            set[i]=i; // assign each node to its own set
        }
        PriorityQueue q = new PriorityQueue(maxVertices*maxVertices); // create priority queue
        for (int i=0; i<activeVertices; i++) { // copy edges to queue
            for (int j=0; j<activeVertices; j++) { 
                if (edges[i][j]!=0) {
                    q.enqueue(new Edge(vertices[i],vertices[j],edges[i][j]));
                }
            }
        }

        while (!q.isEmpty()) { // iterate over all edges in priority order
            Edge e = q.dequeue(); // consider next edge
            if (set[e.source.graphIndex]!=set[e.destination.graphIndex]) { // skip edges not connecting different sets
                mst.addEdge(e.source, e.destination, e.weight); // add edge to MST
                System.out.println("adding "+e);
                int setToMerge=set[e.destination.graphIndex]; // rename nodes from "other" set
                for (int i=0; i<activeVertices; i++) {
                    if (set[i]==setToMerge) { // find nodes from "other" set
                        set[i]=set[e.source.graphIndex]; // reassign nodes
                    }
                }
            }
        }
        return mst;
    }

    public void print() {
        System.out.format("    ");
        for (int i=0; i<activeVertices; i++) {
            System.out.format("%3s ", vertices[i].name);
        }
        System.out.format("\n");
        for (int j=0; j<activeVertices; j++) {
            System.out.format("%3s ", vertices[j].name);
            for (int i=0; i<activeVertices; i++) {
                System.out.format("%3d ", edges[i][j]);
            }
            System.out.format("\n");
        }
    }
}

If you don't want to have duplicated entries and are not bound to arrays you can also use a Set. And if the order matters then a LinkedHashSet — iberbeu
– iberbeu, Commented Nov 20, 2013 at 17:31
Also note that you're traversiong the whole array everytime you want to check if a vertex exists! This would be much faster (as in O(1) instead of O(n)) if you used something like a HashSet. If you need to preserve insertion order, go for a LinkedHashSet — Miquel
– Miquel, Commented Nov 20, 2013 at 17:39
@Mikecat119 This constraint is insane. Note that you can use a Set inside your class, while maintaining an interface to the outside world that gives and takes arrays. — Ingo
– Ingo, Commented Nov 20, 2013 at 18:16

forgivenson · Accepted Answer · 2013-11-20 18:15:55Z

1

First, you should be using equals instead of ==. You should write a proper equals method in your Vertex class (use Google to find plenty of tutorials on how to do this).

For example, if you wanted two Vertex objects to be considered equal only when their names were the same, then your equals method would look something like this:

public boolean equals(Object obj) {
    if(obj == null) {
        return false;
    }

    if(obj instanceof Vertex) {
        Vertex otherVertex = (Vertex) obj;
        if(this.name.equals(otherVertex.name)) {
            return true;
        }
    }
    return false;
}

If you wanted to compare graphIndex as well, then you would need to check that in the equals method as well.

Assuming you have a proper equals method in your Vertex class, the simplest solution would be to use the ArrayUtils.contains method, from the Apache Commons library (Apache Commons has TONS of useful methods, which can save you a lot of time. You should check it out). This method takes in an array and an Object, and checks if the array contains the object or not.

edited Nov 20, 2013 at 18:15

answered Nov 20, 2013 at 18:08

forgivenson

4,4332 gold badges21 silver badges28 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

scottb Over a year ago

The optimization if (obj == null) return false; is redundant. Since null is never an instanceof Vertex, that comparison will always return false (it is a property of the instanceof operator specified in the JLS).

forgivenson Over a year ago

I never knew instanceof handled null. Thanks for the info!

T.J. Crowder · Accepted Answer · 2013-11-20 17:27:18Z

0

You're always checking vertices[1] against vertices[0] and adding based on the result. You're not checking for v, and not actually looking in the whole array.

If an == check (identity, not equivalence) is really what you want, then:

public void addVertex(Vertex v) {
    if (activeVertices >= maxVertices) {
        System.out.println("Graph full");
        return;
    }
    for(int i=0; i<vertices.length; i++) {
        if(vertices[i] == v){
            // Already have it
            return;
        }
    }
    vertices[activeVertices] = v; // add vertex to list of vertices
    v.graphIndex = activeVertices; // record index of vertex in graph
    activeVertices++;  // increment vertex count
}

If you want equivalence instead, replace

if(vertices[i] == v){

with

if(v.equals(vertices[i])){

Side note: Based on your having an activeVertices variable, I suspect you may be better off with ArrayList<Vertex> rather than Vertex[]. That would also give you the contains method (which uses equals), which may be able to replace your loop (if you want an equals, not ==, check).

answered Nov 20, 2013 at 17:27

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

2 Comments

Mikecat119 Over a year ago

I tried your implementation and "Graph full" is printed out the same number of times as the number of elements I am inserting into the array.(50 elements, "Graph full" is printed 50 times). Then the first element I wish to insert into the array is inserted until it fills up the entire array. So my array contains the first piece of data in every element.

T.J. Crowder Over a year ago

@Mikecat119: And did you look at equals as I mentioned above?

scottb · Accepted Answer · 2013-11-20 18:31:04Z

Whenever you write a value class, i.e. a class that represents a value or quantity of something, you should always override the following methods for your class:

equals(Object o);
hashCode();

Not all classes are value classes. Some represent system resources and others represent actions or processes, but whenever a class is written as an abstraction for a collection of data you should always consider writing the above methods.

The reason is straightforward. Whereas Java primitives have only value, Java reference types (which include all the instances of classes you write yourself) have both value and location. This confers the properties of both equality and identity to reference types and they are very different.

By default, the equals() method in the Object class performs an identity comparison and NOT an equality comparison ... and it's a good thing too. Because any subclass of Object can have vastly different notions of "how instances can be considered equal" there is no straightforward way that Object could have a superclass method that would test equality for any arbitrary Java object. In contrast, it is always straightforward to test for identity. If any two references indicate the same location, then their objects are identical. This exemplifies the different notions of equality and identity.

You need to be able to test whether your Vertex instances are equal and not whether they are identical. For this reason, you really do need to override the equals(Object o) method. If you also override hashCode() (which you should), then you may be able to store your vertices in a HashSet, which would guarantee that no two vertices would be equal.

Collectives™ on Stack Overflow

Avoiding duplicate entries in an array

3 Answers 3

2 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related