3
\$\begingroup\$

I have written a program that takes a linked list and then converts it to a stack.

I would like a code review for my program. Thanks in advance.

Stack.java

/**
 * Created by Ninan John J P on 2/8/2016.
 */

public class Stack {

    int stackArray[];
    int topOfStack=-1;
    LinkedList source;
    int stackSize;

    public Stack(LinkedList l1) {

        source=l1;
        stackSize=l1.count;
        stackArray= new int[stackSize];

    }


    private void put(int data) {

        stackArray[++topOfStack]=data;

    }

    private int pop(){

        return stackArray[topOfStack--];

    }

    public static void main(String...a){

        LinkedList l1= new LinkedList();
        l1.getInitialData();
        l1.create();

        Stack s1= new Stack(l1);
        Node currentNode=l1.head;

        s1.put(Integer.parseInt(currentNode.getData()));

        while(currentNode.getNextNode()!=null) {

            currentNode=currentNode.getNextNode();
            s1.put(Integer.parseInt(currentNode.getData()));

        }

        System.out.println("The Stack is:");

        while(s1.topOfStack>=0)
            System.out.print(s1.pop() + "\t");

    }

}

LinkedList.java

/**
 * Created by Ninan John J P on 2/3/2016.
 */

import java.util.Scanner;

public class LinkedList {

    boolean createFlag=false;
    int noOfInitialNodes;
    Node head;
    int count=0;

    private void findElement() {

        String toFind;
        boolean found=false;
        int index=0;
        Scanner in = new Scanner(System.in);
        System.out.println("Enter Number to Find");
        toFind=in.next();

        if(count<=0)
            System.out.println("List is Empty");

        else {

            Node currentNode=head;

            while(currentNode!=null&&!found){

                if(currentNode.getData().equals(toFind)){

                    System.out.println("Found At " + (index+1));
                    found = true;

                }
                else{

                    currentNode=currentNode.getNextNode();
                    index++;

                }
            }
        }

        if (!found)
            System.out.println("Not Found");

    }

    private Node findElementNode(String toFind) {

        Node currentNode;

        if(count<=0)
            System.out.println("List is Empty");

        else {

            currentNode=head;

            while(currentNode!=null){

                if(currentNode.getData().equals(toFind))
                    return currentNode;

                else if(currentNode.getNextNode()!=null) {

                    if (currentNode.getNextNode().getData().equals(toFind))
                        return currentNode;

                }

                currentNode=currentNode.getNextNode();
            }

        }

        return null;

    }

    private void deleteList() {

        head.setNext(null);
        head=null;

    }

    private void deleteNode() {

        Scanner in= new Scanner(System.in);
        String numToDelete;
        System.out.println("Enter the number to delete");
        numToDelete=in.nextLine();
        Node currentNode;

        currentNode=findElementNode(numToDelete);

        if(currentNode==null)
            System.out.println("Element Not found");

        else if(currentNode==head)
            head = currentNode.getNextNode();

        else if (currentNode.getNextNode()!=null) {

            if(currentNode.getNextNode().getNextNode()!=null)
                currentNode.setNext(currentNode.getNextNode().getNextNode());

            else
                currentNode.setNext(null);

        }

    }

    private void print() {

        Node currentNode= head;

        if(head==null) {

            System.out.println("List Does not Exist");
            main();

        }

        System.out.print(currentNode.getData());

        while(currentNode.getNextNode()!=null){

            currentNode=currentNode.getNextNode();
            System.out.print("-->" + currentNode.getData());

        }

    }

    private void insertNew() {

        Scanner in = new Scanner(System.in);
        String data;
        System.out.println("Enter Data for Node:");
        data=in.next();

        Node tempNode= new Node(data);
        Node currentNode=head;

        if(currentNode!=null){

            while(currentNode.getNextNode()!=null)
                currentNode=currentNode.getNextNode();

            currentNode.setNext(tempNode);

        }

        count++;

    }

    public void insertAtPos(){

        Scanner in = new Scanner(System.in);
        String data;
        int index;

        if(head!=null) {

            System.out.println("Enter Data for Node:");
            data = in.next();

            System.out.println("Enter the position");
            index = in.nextInt();

            Node tempNode = new Node(data);
            Node currentNode = head;

            if (currentNode != null) {

                for (int i = 1; i < index - 1 && currentNode.getNextNode() != null; i++)
                    currentNode = currentNode.getNextNode();

                if (index != 1) {

                    tempNode.setNext(currentNode.getNextNode());
                    currentNode.setNext(tempNode);

                }

                else {

                    tempNode.setNext(head);
                    head = tempNode;

                }

                count++;

            }
        }
    }

    protected void create() {

        if(!createFlag) {

            createFlag=true;
            int i = 0;
            Scanner in = new Scanner(System.in);
            String data;
            System.out.println("Enter Data for head Element:");
            data = in.next();

            head = new Node(data);
            count++;

            while (i < noOfInitialNodes-1) {

                insertNew();
                i++;

            }
        }

        else
            System.out.println("List is aready Created... Insert new elements");

    }

    protected void getInitialData() {

        Scanner in = new Scanner(System.in);
        System.out.print("\nEnter Initial Number of Nodes:");
        noOfInitialNodes=in.nextInt();

    }


    public static void main(String...a){

        LinkedList l1= new LinkedList();
        Scanner in = new Scanner(System.in);
        int choice;

        do {

            System.out.println("\nEnter a Number between 1-6");
            System.out.println("1}Insert At Position\n2)Delete Node\n3)Delete List\n4)Find\n5)Create\n6)Print\n7)Exit\n8)Insert At End");

            choice= in.nextInt();

            switch (choice) {

                case 1:

                    l1.insertAtPos();
                    l1.print();
                    break;

                case 2:

                    l1.deleteNode();
                    l1.print();
                    break;

                case 3:

                    l1.deleteList();
                    break;

                case 4:

                    l1.findElement();
                    break;

                case 5:
                    l1.getInitialData();
                    l1.create();
                    l1.print();
                    break;

                case 6:
                    l1.print();
                    break;

                case 7:
                    System.exit(0);
                    break;

                case 8:
                    l1.insertNew();
                    l1.print();
                    break;

            }

        }while(choice<9);

    }

}

Node.java

/**
 * Created by Ninan John J P on 2/6/2016.
 */


class Node{

    String data;
    Node next;

    public Node(String objdata){

        next= null;
        data=objdata;

    }

    public Node(String objData, Node nextNode){

        next=nextNode;
        data=objData;

    }

    public Node getNextNode(){

        return next;

    }

    public String getData(){

        return data;

    }

    public void setData(String objData){

        data=objData;

    }

    public void setNext(Node nextNode){

        next= nextNode;

    }

}
\$\endgroup\$
  • \$\begingroup\$ this is reinventing the wheel, right? You're doing this as homework / learning exercise? There's classes in Java for LinkedList and Stack already, and they're in the standard library. \$\endgroup\$ – Pimgd Jun 8 '16 at 13:49
  • \$\begingroup\$ yes, this is done as a learning exercise. \$\endgroup\$ – leoOrion Jun 8 '16 at 13:50
3
\$\begingroup\$

Mains in classes

I don't like having main methods in non-application classes, which is what you've done in both your linked list and stack classes. Having a main method in the class blurs the boundary of what the class is responsible for. At a minimum it's responsible for being a 'stack' and being an application entry point, it feels wrong.

Public interface

Think about what it is you're trying to model and what that means for the interface to your classes and what the classes are responsible for. When I think of a stack class, I think of something that is going to let me push objects onto it and pop them back off. So for me, a Stack would at minimum have public push and pop methods. Yours doesn't, the methods are private. I suspect this is because you're only using them from your main which is in the same class.

Separation of Concerns

You linked list is asking the user to supply data, then using that to create nodes and add them to the list. This is too much responsibility for the one class and it makes it difficult for you to reuse the list in other ways. If you had a list builder that was responsible for driving the process of collecting input from the user in order to construct the list via it's public interface, this would lead to a more focused implementation of the list. This would for example allow you to reuse the list in order to construct the stack class. A linked list is an easy structure for implementing a stack. To achieve stack like behaviour, you always add new elements to the head of the list (push) and only allow the first element to be removed from the list (pop). You can't currently implement stack like this however because of the user interaction embedded in your linked list class.

Bounds Checking

Currently your push method on your stack doesn't check if it's going to overflow the internal array. This is OK at the moment, because the method is private, however as I've said it's odd for a stack class not to have public push/pop methods. If they do become public then they need to start doing bounds checking to prevent an overflow.

Only implement what you need

As far as I can tell, you don't use your setData method on the Node class. Try to avoid creating extra work for yourself by creating methods that you're not calling. It just gives your more friction if you decide you need to refactor the way your code works.

\$\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.