TwilightSparkleTheGeek
TwilightSparkleTheGeek

Reputation: 223

How do I merge both of these functions together?

public boolean insert(K key, V value) {
    if (contains(key)) { //if the binary tree contains it don't insert it.
        return false;
    }
    if (rNode == null) { //if the root node is empty, there is nothing in the tree
        //create a new DictionaryNode.
        rNode = new DictionaryNode<K, V>(key, value);
        curNode = rNode;
    } else {//if the above aren't true then you can insert it.
        placeNode(key, value, rNode, null, false); //use private method placeNode
    }
    changeCounter++;
    currentSize++;
    return true;
}//end insert

This is the other function. I want to be able to do all of what placeNode does in my insert method. I want to be able to get rid of my placeNode method.

private void placeNode(K key, V value, DictionaryNode<K, V> node, DictionaryNode<K, V> parent, boolean nodeLeft) {
    if (node == null) {
        if (nodeLeft) {
            parent.lChild = new DictionaryNode<K, V>(key, value);
        } else {
            parent.rChild = new DictionaryNode<K, V>(key, value);
        }
    } else if (((Comparable<K>) key).compareTo(node.key) < 0) {
        placeNode(key, value, node.lChild, node, true);
    } else {
        placeNode(key, value, node.rChild, node, false);
    }
}//end placeNode

Upvotes: 0

Views: 70

Answers (2)

robbmj
robbmj

Reputation: 16526

private DictionaryNode<K, V> curNode = rNode;    
public boolean insert(K key, V value) {
            if (contains(key)) { // if the binary tree contains it don't insert it.
                return false;
            }
            if (rNode == null) { // if the root node is empty, there is nothing in
                                    // the tree
                                    // create a new DictionaryNode.
                rNode = new DictionaryNode<K, V>(key, value);

            } else {// if the above aren't true then you can insert it.

                int c = ((Comparable<K>)key).compareTo(curNode.key);


                if (c < 0) {
                    if (curNode.lChild == null) {
                        curNode.lChild = new DictionaryNode<K, V>(key, value);
                    }
                    else {
                        curNode = curNode.lChild;
                        return insert(key, value);
                    }
                }
                else {
                    if (curNode.rChild == null) {
                        curNode.rChild = new DictionaryNode<K, V>(key, value);
                    }
                    else {
                        curNode = curNode.rChild;
                        return insert(key, value);
                    }
                }

            }
            curNode = rNode;
            changeCounter++;
            currentSize++;
            return true;
        }

edit --

assuming K and V come from a class declaration they should be declared as

public SomeClass<K extends Comparable<K>, V extends Comparable<V> { ... }

as Makoto suggested.

Upvotes: 1

Makoto
Makoto

Reputation: 106460

There's nothing to be gained except a headache from merging these two functions together. They do distinct operations, and as such, that should be separated out into smaller chunks of code wherever possible.

The splitting of the methods is fine. Don't attempt to merge them.

The only real thing you could do is replace your method call with the exact body of the method you wish to remove...but that would make things extremely complex.

As an aside, your cast ((Comparable<K>) key could produce a ClassCastException. If the type bound to K isn't comparable, then at runtime, you would have a major problem. Generics weren't meant to be used in that manner - you want to provide compile-time safety of types.

Luckily, you can fix this issue by adding the upper bounds to your types in your class:

public class DictionaryNode<K extends Comparable<K>, V> {
    // implementation to follow
}

Upvotes: 2

Related Questions