5
\$\begingroup\$

I am trying to make a ferry simulator with one ferry that works like a stack and another like a queue. I have a working class for stacks and queues with pop/push; I am trying to figure out how to simply this code for the ferry class.

import java.util.Scanner;



public class Ferry {

    static Stack ferry1 = new Stack();
    static Queue ferry2 = new Queue();
    Stack ferryStack = new Stack();
    static int car1, car2, car3, car4, car5, car6, car7, car8, car9, car10, car11, car12, car13, car14, car15, car16, car17, car18, car19, car20;
    static int getTag(){

        Scanner tag = new Scanner(System.in);
        int carID;

        System.out.print("Enter in a tag number.");
        carID = tag.nextInt();
        return carID;
    }
    public static Queue ferry2Loading(){
        car1 = getTag();
        car2 = getTag();
        car3 = getTag();
        car4 = getTag();
        car5 = getTag();
        car6 = getTag();
        car7 = getTag();
        car8 = getTag();
        car9 = getTag();
        car10 = getTag();
        car11 = getTag();
        car12 = getTag();
        car13 = getTag();
        car14 = getTag();
        car15 = getTag();
        car16 = getTag();
        car17 = getTag();
        car18 = getTag();
        car19 = getTag();
        car20 = getTag();

        ferry2.queuePush("car1");
        ferry2.queuePush("car2");
        ferry2.queuePush("car3");
        ferry2.queuePush("car4");
        ferry2.queuePush("car5");
        ferry2.queuePush("car6");
        ferry2.queuePush("car7");
        ferry2.queuePush("car8");
        ferry2.queuePush("car9");
        ferry2.queuePush("car10");
        ferry2.queuePush("car11");
        ferry2.queuePush("car12");
        ferry2.queuePush("car13");
        ferry2.queuePush("car14");
        ferry2.queuePush("car15");
        ferry2.queuePush("car16");
        ferry2.queuePush("car17");
        ferry2.queuePush("car18");
        ferry2.queuePush("car19");
        ferry2.queuePush("car20");
         return ferry2;
    }
    public static Stack ferry1Loading(){
        car1 = getTag();
        car2 = getTag();
        car3 = getTag();
        car4 = getTag();
        car5 = getTag();
        car6 = getTag();
        car7 = getTag();
        car8 = getTag();
        car9 = getTag();
        car10 = getTag();
        car11 = getTag();
        car12 = getTag();
        car13 = getTag();
        car14 = getTag();
        car15 = getTag();
        car16 = getTag();
        car17 = getTag();
        car18 = getTag();
        car19 = getTag();
        car20 = getTag();

        ferry1.stackPush(car1);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car2);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car3);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car4);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car5);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car6);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car7);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car8);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car9);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car10);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car11);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car12);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car13);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car14);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car15);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car16);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car17);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car18);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car19);
        System.out.println(ferry1.stackPeek());
        ferry1.stackPush(car20);
        System.out.println(ferry1.stackPeek());

        return ferry1;
    }

public static void main(String args[]){

    ferry1Loading();
    ferry2Loading();    
    //System.out.println("CARS ON THE FERRY "+ferry1+"");
        //ferry1.push("s5gsg");

//      
//      Stack ferryStack = new Stack(20);
//      System.out.println(ferryStack.peek());
//      ferryStack.push("2b");
//      System.out.println(ferryStack.peek());
//      ferryStack.push("665sg");
//      System.out.println(ferryStack.peek());
//      ferryStack.push("12sgs");
//      System.out.println(ferryStack.peek());
//      ferryStack.push("3sgsg2");
//      System.out.println(ferryStack.peek());
//      ferryStack.pop();
//      ferryStack.push("2sg0");
//      System.out.println(ferryStack.peek());

    }
}

Stack Code:

public class Stack {
    int maxSize;
    int top;
    String arr[];

    public Stack(){
        int n = 0;
        maxSize = n;
        arr = new String[maxSize];
        top = 0;
    }
    public void stackPush(String str){
        if(top < maxSize){
        arr[top] = str;
        top++;
        }else{
        System.out.println("Error Stack Overflow!!");
    }
    }
    public String stackPop(){
        if(!this.empty()){
        String popped = this.peek();
        arr[top-1] = null;
        top--;
        return popped;
    }else{
        return null;
    }
        }
    public String peek(){
        if(top > 0){
        return arr[top-1];
    }else{
        return null;
    }
    }
    public boolean empty(){
    if (top == 0){
        return true;
    }else{

        return false;
    }
    }
}

Queue Code:

public class Queue {
int maxSize;
int top;
int rear;
String arr[];

public Queue(){
    int n = 0;
    maxSize = n ;
    arr = new String[maxSize];
    top = -1;
    rear = 0;
}

public void queuePush(String cars){
    if(top < maxSize - 1){
        top++;
        arr[top] = cars;
        //display();
//  }else{
//      System.out.println("Error Queue Overflow!!");
    }
}
public void queuePop(){
    if(top >= rear){
        rear++;
        //display();
    }else{
        System.out.println("Error");
    }
//}
//public void display() {  
//  if (top >= rear) {  
//   System.out.println("Elements in Queue : ");  
//   for (int i = rear; i <= top; i++) {  
//    System.out.println(arr[i]);  
//   }  
//  }  
// }  
//  
// public static void main(String[] args) {  
//  Queue queueDemo = new Queue(20);  
//  queueDemo.pop();  
//  queueDemo.push("23");  
//  queueDemo.push("2");  
//  queueDemo.push("73");  
//  queueDemo.push("21");  
//  queueDemo.pop();  
//  queueDemo.pop();  
//  queueDemo.pop();  
//  queueDemo.pop();  
// }  

}
}
\$\endgroup\$
3
  • \$\begingroup\$ Welcome to Code Review! I hope you get some helpful answers. \$\endgroup\$
    – SirPython
    Commented Oct 21, 2015 at 23:32
  • \$\begingroup\$ Why does Stack store String while Queue store int? \$\endgroup\$ Commented Oct 22, 2015 at 1:54
  • \$\begingroup\$ I didnt even pay attention, so I need to make both of them int int \$\endgroup\$ Commented Oct 22, 2015 at 2:02

2 Answers 2

10
\$\begingroup\$

A single ferry?!

Why is this Ferry class (mostly) static? There is such thing as multiple ferries. And, since this information that you are putting inside the Ferry class isn't going to be useful to anything but this one class, then you should turn this into a class that is instantiated and stores data in it's own instance.

As Quill noted in a comment:

ferry1Loading, ferry2Loading: doesn't that imply more than one ferry?

You should refactor your class so you are instantiating two separate ferries and loading them, rather than creating these as fields inside the class itself.


Scanners everywhere!

static int getTag(){

    Scanner tag = new Scanner(System.in);
    int carID;

    System.out.print("Enter in a tag number.");
    carID = tag.nextInt();
    return carID;
}

Each time this method is run, a new Scanner is instantiated. This is wasteful and unnecessary. You should create a single Scanner, stick it in a field, and only use that Scanner to get input (make static if you do not follow the above advice)

private static Scanner input;
...
public static void main(String[] args) {
    input = new Scanner(System.in);
}

If you follow the recommendation above about making this into a class that is instantiated, then you can, in the constructor, have an option for specifying what stream the Scanner should read from. That would allow for easier unit testing in the future.


DRY DRY DRY DRY...

Don't repeat yourself.

    car1 = getTag();
    car2 = getTag();
    car3 = getTag();
    car4 = getTag();
    car5 = getTag();
    car6 = getTag();
    car7 = getTag();
    car8 = getTag();
    car9 = getTag();
    car10 = getTag();
    car11 = getTag();
    car12 = getTag();
    car13 = getTag();
    car14 = getTag();
    car15 = getTag();
    car16 = getTag();
    car17 = getTag();
    car18 = getTag();

All this code and the other code blocks like this are ugly looking. You have soo many statements just like this, with the only change being the variable/field acted upon.

You should, to fix this, change all these carX fields into a single array of cars. Then, when you are doing repeated operations like this, you can simply iterate over the array.

Here is an example:

private int[] cars; /* set to new int[20] in constructor */
....
for(int i = 0; i < cars.length; i++) {
    cars[i] = getTag();
}

Follow this same idea of using loops for the rest of your repeated code.

\$\endgroup\$
3
  • \$\begingroup\$ ferry1Loading, ferry2Loading: doesn't that imply more than one ferry? \$\endgroup\$
    – Quill
    Commented Oct 21, 2015 at 23:41
  • \$\begingroup\$ The second snippet in "Scanners Everywhere" section can't really run. You can't use non-static variables in static methods, even in its own class. \$\endgroup\$ Commented Oct 22, 2015 at 2:07
  • \$\begingroup\$ for(int car : cars) { car = getTag(); } won't work as int cars is a separate temporary variable and not an alias for an array index. \$\endgroup\$
    – Johnbot
    Commented Oct 22, 2015 at 8:20
5
\$\begingroup\$
static int car1, car2, car3, car4, car5, car6, car7, car8, car9, car10, car11, car12, car13, car14, car15, car16, car17, car18, car19, car20;

You really need this? Use an array:

static int[] cars = new int[20];

20 is also a magic number:

public static final int NUMBER_OF_CARS = 20;

I really don't see the point of the Ferry class here. What is its point? From what I understand, I would completely redesign it:

import java.util.Scanner;

public class Ferry {

    public static final int NUMBER_OF_CARS = 20;

    private static Stack ferry1 = new Stack();
    private static Queue ferry2 = new Queue();
    private static int[] cars = new int[NUMBER_OF_CARS];

    static int getTag() {

        Scanner tag = new Scanner(System.in);
        int carID;

        System.out.print("Enter in a tag number.");
        carID = tag.nextInt();
        return carID;
    }

    public static Queue ferry2Loading() {
        for (int i = 0; i < NUMBER_OF_CARS; i++) {
            cars[i] = getTag();
            ferry2.queuePush("car" + (i + 1));
        }
        return ferry2;
    }

    public static Stack ferry1Loading() {
        for (int i = 0; i < NUMBER_OF_CARS; i++) {
            cars[i] = getTag();
            ferry1.stackPush(cars[i]);
            System.out.println(ferry1.stackPeek());
        }
        return ferry1;
    }

    public static void main(String args[]) {
        ferry1Loading();
        ferry2Loading();
    }
}

What I did:

  • I used loops. Loops are very useful in programming; use them.
  • I added private modifiers. You don't want other classes peeking at those, do you?

In , it is usually bad practice to use static so often, as is a OOP (Object Oriented Programming) language. I have no idea what your class is doing, so I can't redesign your code to be more OOP.

EDIT: After providing me with some information, I came up with this:

EDIT: With some more information, I would do:

import java.util.Scanner;

public abstract class Ferry {

    public static final int NUMBER_OF_CARS = 20;

    protected int[] cars = new int[NUMBER_OF_CARS];

    protected final Scanner tag = new Scanner(System.in);

    protected int getTag() {
        System.out.print("Enter in a tag number.");
        int carID = tag.nextInt();
        return carID;
    }

    public abstract void loadFerry();

    public abstract int[] unloadFerry();

}

public final class StackFerry extends Ferry {

    private Stack ferry = new Stack();

    @Override
    public void loadFerry() {
        for (int i = 0; i < Ferry.NUMBER_OF_CARS; i++) {
            cars[i] = getTag();
            ferry.stackPush(cars[i]);
            System.out.println(ferry.stackPeek());
        }
    }

    @Override
    public int[] unloadFerry() {
        int[] result = new int[Ferry.NUMBER_OF_CARS];
        for (int i = 0; i < Ferry.NUMBER_OF_CARS; i++) {
            result[i] = ferry.popStack();
        }
        return result;
    }

    public Stack getFerry() {
        return ferry;
    }

}

public final class QueueFerry extends Ferry {

    private Queue ferry = new Queue();

    @Override
    public void loadFerry() {
        for (int i = 0; i < Ferry.NUMBER_OF_CARS; i++) {
            cars[i] = getTag();
            ferry.queuePush(cars[i]);
            System.out.println(ferry.stackPeek());
        }
    }

    @Override
    public int[] unloadFerry() {
        int[] result = new int[Ferry.NUMBER_OF_CARS];
        for (int i = 0; i < Ferry.NUMBER_OF_CARS; i++) {
            result[i] = ferry.popQueue();
        }
        return result;
    }

    public Queue getFerry() {
        return ferry;
    }

}

And add your main method in another class in another file called Main. That would look like (if you want to time it):

public class Main {

    public static void main(String args[]) {
        StackFerry stackFerry = new StackFerry();
        long startTime = System.nanoTime();
        stackFerry.loadFerry();
        long endTime = System.nanoTime();
        System.out.println(endTime - startTime);
        QueueFerry queueFerry = new QueueFerry();
        startTime = System.nanoTime();
        queueFerry.loadFerry();
        endTime = System.nanoTime();
        System.out.println(endTime - startTime);
        startTime = System.nanoTime();
        stackFerry.unloadFerry();
        endTime = System.nanoTime();
        System.out.println(endTime - startTime);
        startTime = System.nanoTime();
        queueFerry.unloadFerry();
        endTime = System.nanoTime();
        System.out.println(endTime - startTime);
    }

}
\$\endgroup\$
6
  • \$\begingroup\$ What I am trying to do is test my classes by writing a driver program, ferry.java, to simulate loading and unloading the two ferries. In the ferry class I want to be able to create a ferry that is a stack object create a ferry that is a queue object. Then write code to load/unload about 20 cars onto each ferry. I want to beable to write code to see how long does it take to load/unload each type of ferry? So I am trying to make two ferries \$\endgroup\$ Commented Oct 22, 2015 at 0:17
  • \$\begingroup\$ @ShemaMartin I have edited my answer. What do you mean by "I want to beable to write code to see how long does it take to load/unload each type of ferry"? Do you mean how long the code takes to execute? Or do you mean each car has a specified time of loading that is added up? \$\endgroup\$ Commented Oct 22, 2015 at 1:24
  • \$\begingroup\$ Both! I am trying to figure out how to write code to see how long it takes to excute loading and unloading and to see how long each car in on the ferries. Does that make since? \$\endgroup\$ Commented Oct 22, 2015 at 1:39
  • \$\begingroup\$ @ShemaMartin I understand half... I can do the execution part, but how do you intend to figure out how much each car takes to load and unload? \$\endgroup\$ Commented Oct 22, 2015 at 1:40
  • \$\begingroup\$ Im going to update my post with the code I have for my Stack/Queue. \$\endgroup\$ Commented Oct 22, 2015 at 1:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.