-4

We are practicing for an exam and are trying to find the minimum value of a linked list in java. This algorithm keeps returning the last element of the list instead of the minimum. 

public class minMax {

    element head;

    public void MinMax(){
        this.head = null;
    }

    public void addElement(element el){
        element reference = this.head;
        this.head = el;
        element nxt = this.head.getNext();
        nxt= reference;
    }

    public int findMin(){
        int min = this.head.getValue();
        element current = this.head;
        while (current != null) {
            if(current.getValue() < min){
                System.out.println("found min");
                min = current.getValue();
            }
            current = current.getNext();
        }
        return min;
    }

    public static void main(String[] args) {
        element a = new element(5,null);
        element b = new element(55, null);
        element c = new element(45, null);
        minMax list= new minMax();
        list.addElement(a);
        list.addElement(b);
        list.addElement(c);

        int min = list.findMin();
        System.out.println(min);
    }

}
mikej
  • 65,295
  • 17
  • 152
  • 131
HELPUS
  • 1

2 Answers2

1

The main problem is with this part:

element nxt = this.head.getNext();
nxt= reference;

This doesn't change the value of next inside head in the way that you're expecting. It just makes the nxt variable refer to reference.

You haven't included the code for your Element class, but you probably want to update next directly e.g.

this.head.setNext(reference);

Also this line:

public void MinMax() {

is not defining a constructor for your class, as you're probably expecting it to because the case of the name MinMax is different to the name of the class minMax. A constructor also doesn't have a return type so to fix this rename your class MinMax (to follow Java naming conventions) and then remove the void from the constructor definition.

mikej
  • 65,295
  • 17
  • 152
  • 131
0

Based on your demo, I just tested it locally and make some modifications.

  • Using Comparable to enable you to replace the type easily as long as the type implemented the Comparable interface (to find the minimum, you have to compare);
  • Using head as the sentry to make the adding and deleting (if you need to delete) easier;

By the way, in java you'd better use Uppercase prefix for the class name, so your class name element should be replace by Element. And actually you are encapsulating your class in a good way as a beginner.

Here is the code: 

public class HelloWorld {

    Node head; // not store any value, just used to link the nodes;

    public Comparable findMin() {
        if (head == null || head.next == null) {
            return null;
        }
        Comparable min = head.next.value;
        Node p = head.next.next;
        while(p != null) {
            if (min.compareTo(p.value) > 0) min = p.value;
            p = p.next;
        }
        return min;
    }

    public void add(Node node) {
        if (head == null) {
            head = new Node(null, node);
        } else {
            node.next = head.next;
            head.next = node;
        }
    }

    public static void main(String... args) {
        HelloWorld list = new HelloWorld();
        list.add(new Node(5, null));
        list.add(new Node(45, null));
        list.add(new Node(55, null));
        System.out.println(list.findMin().toString());
    }

    static class Node {
        Comparable value;
        Node next;
        public Node(Comparable theValue, Node theNext) {
            this.value = theValue;
            this.next = theNext;
        }
    }
}

The output is working as you expect. 

5

Hope it helps you ~

Hearen
  • 7,420
  • 4
  • 53
  • 63