Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Intersection

it seems a little like cheating to me to use a HashSet here. I would probably write my own contains method for the linked list (which is bad for performance, but performance is not really the point here).

Other than that:

largerListNode != null

This check seems unnecessary. It's the larger list, after all. items.size() > 0 should always catch this.

Union

The return value should probably not be named intersectionlist :) It also does not do union. The code should look something like this:

    public IntersectionAndUnionLinkedList<T> union(IntersectionAndUnionLinkedList<T> list) {
    // ... in case this list should not be changed, copy it first. Otherwise, this list will be result of the union

    // traverse through the input list. add nodes from input list to beginning of this list
    Node listCurrentNode = list.first;
    while (listCurrentNode != null) {
        // ... if you do not want duplicate elements, check for them. or remove them at the end
        Node listNextNode = listCurrentNode.next;
        Node thisPreviousFirstNode = this.first;
        listCurrentNode.next = thisPreviousFirstNode;
        this.first = listCurrentNode;
        listCurrentNode = listNextNode;
    }
    return this;
}

Another (slower) approach would be to create a new list (the output list), and traverse both input lists one at a time, adding their elements to the new list (if it does not already contain the item).

General

The diamond operator is only supported since Java 7. So I would still use this:

Node<T> node = new Node<T>(item);

instead of this

Node<T> node = new Node<>(item);

if possible.

see also my answer herehere

Intersection

it seems a little like cheating to me to use a HashSet here. I would probably write my own contains method for the linked list (which is bad for performance, but performance is not really the point here).

Other than that:

largerListNode != null

This check seems unnecessary. It's the larger list, after all. items.size() > 0 should always catch this.

Union

The return value should probably not be named intersectionlist :) It also does not do union. The code should look something like this:

    public IntersectionAndUnionLinkedList<T> union(IntersectionAndUnionLinkedList<T> list) {
    // ... in case this list should not be changed, copy it first. Otherwise, this list will be result of the union

    // traverse through the input list. add nodes from input list to beginning of this list
    Node listCurrentNode = list.first;
    while (listCurrentNode != null) {
        // ... if you do not want duplicate elements, check for them. or remove them at the end
        Node listNextNode = listCurrentNode.next;
        Node thisPreviousFirstNode = this.first;
        listCurrentNode.next = thisPreviousFirstNode;
        this.first = listCurrentNode;
        listCurrentNode = listNextNode;
    }
    return this;
}

Another (slower) approach would be to create a new list (the output list), and traverse both input lists one at a time, adding their elements to the new list (if it does not already contain the item).

General

The diamond operator is only supported since Java 7. So I would still use this:

Node<T> node = new Node<T>(item);

instead of this

Node<T> node = new Node<>(item);

if possible.

see also my answer here

Intersection

it seems a little like cheating to me to use a HashSet here. I would probably write my own contains method for the linked list (which is bad for performance, but performance is not really the point here).

Other than that:

largerListNode != null

This check seems unnecessary. It's the larger list, after all. items.size() > 0 should always catch this.

Union

The return value should probably not be named intersectionlist :) It also does not do union. The code should look something like this:

    public IntersectionAndUnionLinkedList<T> union(IntersectionAndUnionLinkedList<T> list) {
    // ... in case this list should not be changed, copy it first. Otherwise, this list will be result of the union

    // traverse through the input list. add nodes from input list to beginning of this list
    Node listCurrentNode = list.first;
    while (listCurrentNode != null) {
        // ... if you do not want duplicate elements, check for them. or remove them at the end
        Node listNextNode = listCurrentNode.next;
        Node thisPreviousFirstNode = this.first;
        listCurrentNode.next = thisPreviousFirstNode;
        this.first = listCurrentNode;
        listCurrentNode = listNextNode;
    }
    return this;
}

Another (slower) approach would be to create a new list (the output list), and traverse both input lists one at a time, adding their elements to the new list (if it does not already contain the item).

General

The diamond operator is only supported since Java 7. So I would still use this:

Node<T> node = new Node<T>(item);

instead of this

Node<T> node = new Node<>(item);

if possible.

see also my answer here

added 205 characters in body
Source Link
tim
  • 25.1k
  • 3
  • 30
  • 76

Intersection

it seems a little like cheating to me to use a HashSet here. I would probably write my own contains method for the linked list (which is bad for performance, but performance is not really the point here).

Other than that:

largerListNode != null

This check seems unnecessary. It's the larger list, after all. items.size() > 0 should always catch this.

Union

The return value should probably not be named intersectionlist :) It also does not do union. The code should look something like this:

    public IntersectionAndUnionLinkedList<T> union(IntersectionAndUnionLinkedList<T> list) {
    // ... in case this list should not be changed, copy it first. Otherwise, this list will be result of the union

    // traverse through the input list. add nodes from input list to beginning of this list
    Node listCurrentNode = list.first;
    while (listCurrentNode != null) {
        // ... if you do not want duplicate elements, check for them. or remove them at the end
        Node listNextNode = listCurrentNode.next;
        Node thisPreviousFirstNode = this.first;
        listCurrentNode.next = thisPreviousFirstNode;
        this.first = listCurrentNode;
        listCurrentNode = listNextNode;
    }
    return this;
}

Another (slower) approach would be to create a new list (the output list), and traverse both input lists one at a time, adding their elements to the new list (if it does not already contain the item).

General

The diamond operator is only supported since Java 7. So I would still use this:

Node<T> node = new Node<T>(item);

instead of this

Node<T> node = new Node<>(item);

if possible.

see also my answer here

Intersection

it seems a little like cheating to me to use a HashSet here. I would probably write my own contains method for the linked list (which is bad for performance, but performance is not really the point here).

Other than that:

largerListNode != null

This check seems unnecessary. It's the larger list, after all. items.size() > 0 should always catch this.

Union

The return value should probably not be named intersectionlist :) It also does not do union. The code should look something like this:

    public IntersectionAndUnionLinkedList<T> union(IntersectionAndUnionLinkedList<T> list) {
    // ... in case this list should not be changed, copy it first. Otherwise, this list will be result of the union

    // traverse through the input list. add nodes from input list to beginning of this list
    Node listCurrentNode = list.first;
    while (listCurrentNode != null) {
        // ... if you do not want duplicate elements, check for them. or remove them at the end
        Node listNextNode = listCurrentNode.next;
        Node thisPreviousFirstNode = this.first;
        listCurrentNode.next = thisPreviousFirstNode;
        this.first = listCurrentNode;
        listCurrentNode = listNextNode;
    }
    return this;
}

General

The diamond operator is only supported since Java 7. So I would still use this:

Node<T> node = new Node<T>(item);

instead of this

Node<T> node = new Node<>(item);

if possible.

see also my answer here

Intersection

it seems a little like cheating to me to use a HashSet here. I would probably write my own contains method for the linked list (which is bad for performance, but performance is not really the point here).

Other than that:

largerListNode != null

This check seems unnecessary. It's the larger list, after all. items.size() > 0 should always catch this.

Union

The return value should probably not be named intersectionlist :) It also does not do union. The code should look something like this:

    public IntersectionAndUnionLinkedList<T> union(IntersectionAndUnionLinkedList<T> list) {
    // ... in case this list should not be changed, copy it first. Otherwise, this list will be result of the union

    // traverse through the input list. add nodes from input list to beginning of this list
    Node listCurrentNode = list.first;
    while (listCurrentNode != null) {
        // ... if you do not want duplicate elements, check for them. or remove them at the end
        Node listNextNode = listCurrentNode.next;
        Node thisPreviousFirstNode = this.first;
        listCurrentNode.next = thisPreviousFirstNode;
        this.first = listCurrentNode;
        listCurrentNode = listNextNode;
    }
    return this;
}

Another (slower) approach would be to create a new list (the output list), and traverse both input lists one at a time, adding their elements to the new list (if it does not already contain the item).

General

The diamond operator is only supported since Java 7. So I would still use this:

Node<T> node = new Node<T>(item);

instead of this

Node<T> node = new Node<>(item);

if possible.

see also my answer here

Source Link
tim
  • 25.1k
  • 3
  • 30
  • 76

Intersection

it seems a little like cheating to me to use a HashSet here. I would probably write my own contains method for the linked list (which is bad for performance, but performance is not really the point here).

Other than that:

largerListNode != null

This check seems unnecessary. It's the larger list, after all. items.size() > 0 should always catch this.

Union

The return value should probably not be named intersectionlist :) It also does not do union. The code should look something like this:

    public IntersectionAndUnionLinkedList<T> union(IntersectionAndUnionLinkedList<T> list) {
    // ... in case this list should not be changed, copy it first. Otherwise, this list will be result of the union

    // traverse through the input list. add nodes from input list to beginning of this list
    Node listCurrentNode = list.first;
    while (listCurrentNode != null) {
        // ... if you do not want duplicate elements, check for them. or remove them at the end
        Node listNextNode = listCurrentNode.next;
        Node thisPreviousFirstNode = this.first;
        listCurrentNode.next = thisPreviousFirstNode;
        this.first = listCurrentNode;
        listCurrentNode = listNextNode;
    }
    return this;
}

General

The diamond operator is only supported since Java 7. So I would still use this:

Node<T> node = new Node<T>(item);

instead of this

Node<T> node = new Node<>(item);

if possible.

see also my answer here