HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Stack from Linked List

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
listfromlinkedstack

Problem

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" + 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.g

Solution

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.

Context

StackExchange Code Review Q#131431, answer score: 3

Revisions (0)

No revisions yet.