5
\$\begingroup\$

I'm trying to design a binary relation interface in Java and then implement it, but I'm not sure if I'm doing it right. I'd really appreciate a little feedback just so I know if I'm way in the wrong direction or not.

This is what I've got so far:

Interface:

public boolean compare(Object that, Object ths);
// compares relations to see if they are equal.

public Set<Y> getY(X value);
//return set of all Y values with relation X

public Set<X> getX(Y value);
//return set of all X values with relation Y

public void clear();
//clear relation of values

public Set<Relation<X, Y>> delY(X value);
//remove all relations containing given Y

public Set<Relation<X, Y>> delX(Y value);
//remove all relations containing given X

public String write(Object that);
//output Relation in string format

public Y addPair (X valX, Y valY);
//adds the entry (X, Y) to the Relation

public Y delPair (X valX, Y valY);
//deletes entry (X, Y) from the Relation

Implementation:

int size;

String[][] basket = new String[16][2];
String[][] bag = basket;



public String addPair(String X, String Y){

    basket[size][0] = X;
    basket[size][1] = Y;

    String pair = basket[size][0] + basket[size][1];
    size++;
    return pair;

}

public boolean compare(String[][] that, String[][] ths){
    that = basket;
    ths = bag;
    boolean equal;

    if(that.equals(ths))
        equal = true;
    else
        equal = false;
    return equal;
}


public Set<String> getY(String X){

    Set<String> Y = new TreeSet<String>();

    for(int i = 0;i<size;i++)
    {
        if(basket[i][0]==X)
            Y.add(basket[i][1]);
    }

    return Y;

}
public Set<String> getX(String Y){

    Set<String> X = new TreeSet<String>();

    for(int i = 0;i<size;i++)
    {
        if(basket[i][1]==Y)
            X.add(basket[i][0]);
    }

    return X;

}

public String write(Object o){


    o = basket;
    String out = "";
    out += o.toString();
    System.out.println("OUT: " + out);
    return out;

}



public void clear(){
 for(int i = 0;i<size;i++)
    {
        if(basket[i][0]!= null){
            basket[i][0] = null;
            basket[i][1] = null;
        }
    }
}

public Set<String> delY(String X, Relation r){
    Set<String> Y = new TreeSet<String>();

    for(int i = 0;i<size;i++)
    {
        if(basket[i][0]==X){
            Y.add(basket[i][1]);
            basket[i][1]=null;
        }
    }

    return Y;

}

public Set<String> delX(String Y){
   Set<String> X = new TreeSet<String>();

    for(int i = 0;i<size;i++)
    {
        if(basket[i][1]==Y){
            X.add(basket[i][1]);
            basket[i][1]=null;
        }
    }


   return X;
}

public String delPair (String X, String Y){
    X = "Orange";
    Y = "Fruit";
    for(int i = 0;i<size;i++)
    {
        if(basket[i][0]==X && basket[i][1]==Y){
            basket[i][1]=null;
            basket[i][0]=null;
        }
    }

     return (X + Y);

}
\$\endgroup\$
2
\$\begingroup\$

There are a couple of things in here that concern me.... but, the most concerning is:

Use .equals() to compare String values

Code like this:

if(basket[i][0]==X){

will fail when you get data that is not String-constant data.

That line should be:

 if(basket[i][0].equals(X)){

This problem is found all over your code.


In addition, there are other problems:

  • your arrays are pre-sized. This code:

    String[][] basket = new String[16][2];
    

    is potentially OK if you are initializing the array, but what if there are more than 16 pairs?

  • it would be better to create a 'Pair' class to contain your two strings, than to use the 2-size array on the basket. Then your basket could be something like:

    StringPair[] basket = new StringPair[SIZE];
    

    your StringPair class could look something like:

    public class StringPair {
        private final String first, second;
    
        public StringPair(String first, String second) {
            this.first = first;
            this.second = second;
        }
    
        ..... getFirst() .....
        ..... getSecond() .....
    
    }
    
  • compare(String[][] that, String[][] ths) is a method that needs no state data. It should be a static method, or a utility method. Alternatively, it should be a simple 1-value method compare(MyClass that) ... which, for what it is worth, would be very similar to the equals(...) method.

    but, that method is pretty broken. It has the two input parameters that and ths, but it then overwrites those values with the internal versions bag and basket.

    that = basket;
    ths = bag;
    boolean equal;
    
    if(that.equals(ths))
        equal = true;
    else
        equal = false;
    return equal;
    

    now, since you set bag = basket earlier, this will always return true.

    Further, that last 5 lines of the method can be replaced with:

    return (that.equals(ths));
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.