4
\$\begingroup\$

I'm an undergraduate student and I have this assignment on a Discrete Event simulator. Currently I have completed to the end of v1.2 here. Attached also, is an algorithmic overview.

enter image description here

I have wrote my code as such to solve the problem at hand. Even though the code feels correct according to the test cases used to test the code, I think there is definitely room for improvement in the design aspect. I came up with this code by drawing the problem out and solving it in a procedural manner and then trying to re-design it in OO. Personally I came from a Python background and now I'm learning Java for this module, so I'm still picking up the ropes on OOP design and getting rid of as much "procedural mindset" as much as possible.

Main

import java.util.Scanner;
import cs2030.simulator.Event;
import cs2030.simulator.Customer;
import cs2030.simulator.EventComparator;
import cs2030.simulator.EventManager;
import cs2030.simulator.ArrivalEvent;

/** 
 * Main class for testing purposes.
 */

public class Main {
    /**
     * Creates an EventManager to simulate the execution of events.
     * This is done by using Scanner object to take in inputs. 
     * required for a RandomGenerator, then pass those arguments to EventManager. 
     * @param args String[] 
     */
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        int seed = sc.nextInt();
        int numServers = sc.nextInt();
        int numCustomer = sc.nextInt();
        double arrivalRate = sc.nextDouble();
        double svcRate = sc.nextDouble();
        sc.close();
        double restRate = 1.0;

        EventManager eventManager = new EventManager(numServers, numCustomer,
            seed,arrivalRate,svcRate,restRate);
        eventManager.doService();
    }
}

EventManager

package cs2030.simulator;

import java.util.PriorityQueue;

/**
 * EventManager class that handles the sequence of Event executions.
 */

public class EventManager {

    /**
     * Array of servers that determines the way ArrivalEvents are processed.
     */
    Server [] servers;

    /**
     * PriorityQueue of events to be cleared by the end of the simulation.
     */
    PriorityQueue<Event> events;

    /**
     * Statistics object to keep track of average waiting time of served customers,
     * number of served customers,
     * and the number of customers that left without being served.
     */
    Statistics statistics = new Statistics();

    /**
     * RandomGenerator object to randomise arrival time as well as
     * service time, which is the time to convert ServedEvent to DoneEvent.
     */
    RandomGenerator gen;


    /**
     * Constructs a EventManager that creates the ArrivalEvents of randomised time.
     * from the number of customers, load them into the PriorityQueue.
     * @param numServers the number of servers to be created.
     * @param numCustomer the number of customers to be served, which is
     * also equal to the number of ArrivalEvents preloaded onto PriorityQueue.
     * @param seed intialising value where the random values are generated from
     * @param arrivalRate arrival rate, mu in RandomGenerator constructor
     * @param svcRate service rate, lambda in RandomGenerator constructor
     * @param restRate rest rate, rho in RandomGenerator constructor
     */
    public EventManager(int numServers, int numCustomer,
        int seed, double arrivalRate, double svcRate, double restRate) {
        this.events = new PriorityQueue<>(new EventComparator());
        this.gen = new RandomGenerator(seed, arrivalRate,svcRate,restRate);
        double time = 0;
        Customer customer = new Customer(time);
        ArrivalEvent tempEvent = new ArrivalEvent(customer,time);
        events.add(tempEvent);
        for (int i = 0;i < numCustomer - 1;i++) {
            double x = gen.genInterArrivalTime();
            time += x;
            customer = new Customer(time);
            tempEvent = new ArrivalEvent(customer,time);
            events.add(tempEvent);
        }
        this.servers = new Server [numServers];
        for (int i = 0;i < numServers;i++) {
            this.servers[i] = new Server();
        }
    }


    /**
     * Processes the full sequence of ArrivalEvents to calculate statistics.
     * This process is split into a few stages,
     * (i)At the start of each loop, get the first event from the PriorityQueue
     * (ii)prints the profile of the event to signal that we start processing it
     * (iii)current event creates the next event, 
     * with some information on the available servers as well as RandomGenerator
     * in case a DoneEvent can be created from the current event.
     * (iv) If applicable, Statistics are also updated after 
     * the creation of the new event, which will also be added to the PriorityQueue. 
     * (v) prints the statistics after the PriorityQueue is cleared.
     */
    public void doService() {
        while (events.size() > 0) {
            Event firstEvent = getFirstEvent();
            System.out.println(firstEvent);
            Event newEvent = firstEvent.getNextEvent(servers,gen);
            if (newEvent != null) {
                newEvent.updateStatistics(statistics);
                events.add(newEvent);
            }

        }
        System.out.println(statistics);
    }

    /**
     * Accesses an event as well as remove it from the PriorityQueue.
     * @return the first event from the PriorityQueue, 
     * according to the Comparator object it was created with.
     */
    public Event getFirstEvent() {
        return events.poll();
    }
}

Server

package cs2030.simulator;

/**
 * Server class that redirects the service of customers according to availability.
 * The status of customer service is seen in terms of events that involves customers.
 */
class Server {
    /**
     * Counter integer that generates ServerID in a non-repetitive way.
     */
    private static int counter = 1;
    /**
     * CustomerID  that allows distinguishing between 2 servers.
     */
    private int serverID;

    /**
     * the first event being resolved by the server.
     */
    private Event served = null;

    /**
     * the second event being resolved by the server.
     */
    private Event wait = null;

    /**
     * Creates a server.
     */
    public Server() {
        this.serverID = counter;
        counter++;
    }

    public int getServerID() {
        return this.serverID;
    }

    /**
     * Causes the first slot inside the Server to be occupied.
     * Or updates the service status of the customer in the first slot.
     * @param newEvent the status of the customer being served
     * in the form of an event.
     */
    public void setServedEvent(Event newEvent) {
        this.served = newEvent;
    }

    /**
     * Causes the second slot inside the Server to be occupied.
     * Or updates the service status of the customer in the second slot.
     * @param newEvent the status of the customer being served
     * in the form of an event.
     */
    public void setWaitEvent(Event newEvent) {
        this.wait = newEvent;
    }

    /**
     * Checks whether the first slot inside the Server has been taken.
     * @return true if first slot has not been taken, false otherwise.
     */
    public boolean canTakeServedEvent() {
        return (served == null && wait == null);
    }

    /**
     * Checks whether the second slot inside the Server has been taken.
     * @return true if the second slot has not been and the first slot is taken,
     * false otherwise.
     */
    public boolean canTakeWaitEvent() {
        return (served != null && wait == null);
    }

    /**
     * Clears up the 2nd slot of the server. 
     * This is done by removing up the 2nd customer to the 1st slot.
     * and replace the 2nd slot with null status.
     */
    public void flushDoneEvent() {
        if (served != null) {
            served = null;
        }
        if (wait != null) {
            served = wait;
            wait = null;
        }
    }

    /**
     * Gets the timestamp at which a customer waiting can expect to be served.
     * @return earliest possible time at which waiting customer can be served
     */
    public double getDoneTime() {
        return this.served.getTime();
    }
}

Customer

package cs2030.simulator;

/**
 * Customer class that holds customer information.
 */

public class Customer {

    /**
     * Counter integer that generates CustomerID in a non-repetitive way.
     */
    private static int counter = 1;

    /**
     * CustomerID  that allows distinguishing between 2 customers.
     */
    private final int customerID;

    /**
     * Time when the customer first arrives.
     */
    private final double time;

    /**
     * Creates Customer.
     * @param time randomised arrival time of customer
     */

    public Customer(double time) {
        this.customerID = counter;
        this.time = time;
        counter++;
    }

    public int getCustomerID() {
        return this.customerID;
    }

    public double getTime() {
        return this.time;
    }

}

EventComparator

package cs2030.simulator;

import java.util.Comparator;

/**
 * EventComparator class to create a comparison criteria for events.
 */
public class EventComparator implements Comparator<Event> {

    /**
     * Compares 2 Events and decides which is smaller, equal or greater.
     * The first key is to check for the earliest time.
     * If there is a tie breaker, customerID is checked instead, 
     * which also hints on the priority of different type of events.
     * @param e1 left event
     * @param e2 right event
     * @return -1 if left event is prioritised over right event. 
     * 0 if there isn't a priority, which will not happen in this case.
     * 1 if right event is prioritised over left event.
     */
    public int compare(Event e1, Event e2)  {
        if (e1.getTime() < e2.getTime()) {
            return -1;
        } else if (e1.getTime() > e2.getTime()) {
            return 1;
        } else if (e1.getCustomerID() < e2.getCustomerID()) {
            return -1;
        } else if (e1.getCustomerID() > e2.getCustomerID()) {
            return 1;
        } else {
            System.out.println("Bug with code, please check");
            return 0;
        }

    }
}

Statistics

package cs2030.simulator;

/**
 * Statistics class to keep track of total waiting time of customers.
 * the number of customers who left without being served.
 * the number of customers who are served.
 */
class Statistics {
    private double waitingTime = 0;
    private int numLeft = 0;
    private int numServed = 0;

    /**
     * Creates Statistics object using the empty constructor.
     */
    public Statistics(){}

    /**
     * Increases the number of customers who are served.
     */
    public void increaseServed() {
        numServed++;
    }

    /**
     * Increases waiting time of customers.
     */
    public void increaseWaitingTime(double time) {
        waitingTime += time;
    }

    /**
     * Increases the number of customers who left without being served.
     */
    public void increaseLeft() {
        numLeft++;
    }

    /**
     * Formats the Statistics to print all information gathered.
     */
    public String toString() {
        double x = waitingTime / numServed;
        return '[' + String.format("%.3f",x) + ' ' + 
            numServed + ' ' + numLeft + ']';
    }
}

Event

package cs2030.simulator;


/**
 * Abstract Event class to enforce polymorphism.
 * Forces its subclasses to have the ability to create the next event
 * and to update statistics.
 */
public abstract class Event {
    /**
     * Customer that the event is involving.
     */
    private final Customer customer;
    /**
     * Time at which the event is created, 
     * which may differ from customer arrival time if 
     * it is only created when it is caused to wait by another preceeding event.
     */
    private final double time;

    /** 
     * Creates an Event.
     * @param customer customer that the event is involving
     * @param time time at which event is created
     */
    public Event(Customer customer, double time) {
        this.customer = customer;
        this.time = time;
    }


    /** 
     * Creates the next event of parent type based on its original type.
     */
    public abstract Event getNextEvent(Server [] servers,RandomGenerator gen);

    /** Modifies information in statistics if required.
     */
    public abstract void updateStatistics(Statistics statistics);

    public Customer getCustomer() {
        return this.customer;
    }

    public int getCustomerID() {
        return this.customer.getCustomerID();
    }

    public double getTime() {
        return this.time;
    }
}

ArrivalEvent

package cs2030.simulator;


/**
 * ArrivalEvent class to simulate the act of a customer arriving.
 */
public class ArrivalEvent extends Event {

    /** 
     * Creates an ArrivalEvent.
     * @param customer customer that the event is involving.
     * @param time time at which event is created.
     */
    public ArrivalEvent(Customer customer, double time) {
        super(customer, time);
    }

    /**
     * Creates the next event based on the availability of servers.
     * The available server will be updated to hold a field of the event 
     * and be involved in the creation of new event.
     * @param servers Array of servers to be checked
     * @param gen RandomGenerator, not used in this case.
     * @return parent class Event, which could be in the form of
     * LeaveEvent, if there are no available servers.
     * ServedEvent, if there exists an available server that is completely free.
     * WaitEvent, if there exists available server and there are no empty servers.
     * null, which won't be reached as it's a Debugging statement.
     */
    public Event getNextEvent(Server [] servers,RandomGenerator gen) {
        Server freeServer = getFreeServer(servers);
        if (freeServer == null) {
            return createLeaveEvent();
        } else if (freeServer.canTakeServedEvent()) {
            ServedEvent newEvent = createServedEvent(freeServer);
            freeServer.setServedEvent(newEvent);
            return newEvent;
        } else if (freeServer.canTakeWaitEvent()) {
            WaitEvent newEvent = createWaitEvent(freeServer);
            freeServer.setWaitEvent(newEvent);
            return newEvent;
        } else {
            System.out.println("Bug in ArrivalEvents");
            return null;
        }
    }

    /**
     * Creates a LeaveEvent not bounded to any server.
     * @return LeaveEvent
     */
    public LeaveEvent createLeaveEvent() {
        return new LeaveEvent(this.getCustomer(),this.getTime());
    }

    /**
     * Creates a ServedEvent bounded to an empty server.
     * @param freeServer the server that is empty.
     * @return ServedEvent.
     */
    public ServedEvent createServedEvent(Server freeServer) {
        return new ServedEvent(this.getCustomer(),this.getTime(),freeServer);
    }

    /**
     * Creates a WaitEvent bounded to a partially occupied server.
     * @param freeServer the server that is partially occupied.
     * @return WaitEvent.
     */
    public WaitEvent createWaitEvent(Server freeServer) {
        return new WaitEvent(this.getCustomer(),this.getTime(),freeServer);
    }

    /**
     * Modifies information in statistics if required.
     * @param statistics Not used in this case.
     */
    public void updateStatistics(Statistics statistics) {
        return;
    }

    /** 
     * Finds the earliest available server based on search results.
     * @param servers Array of servers to be checked.
     * @return Server if an empty server or partially empty server is found
     * null otherwise.
     */
    public Server getFreeServer(Server[] servers) {
        boolean hasFoundSlots = false;
        Server choiceServer = null;
        for (int i = 0; i < servers.length;i++) {
            Server newServer = servers[i];
            if (newServer.canTakeServedEvent()) {
                return newServer;
            } else if (newServer.canTakeWaitEvent() && !hasFoundSlots) {
                choiceServer = newServer;
                hasFoundSlots = true;
            }
        }
        if (hasFoundSlots == false) {
            return null;
        } else {
            return choiceServer;
        }
    }


    /**
     * Formats the ArrivalEvent to print out its profile.
     */
    public String toString() {
        return String.format("%.3f",this.getTime()) + ' ' +
        this.getCustomerID() + " arrives";
    }
}

WaitEvent

package cs2030.simulator;


/**
 * WaitEvent class to simulate the act of customer waiting.
 * for another customer to be served by the same server.
 */
class WaitEvent extends Event {
    /** 
     * Server that the WaitEvent belongs to.
     */
    private Server server;

    /** 
     * Creates an WaitEvent.
     * @param customer customer that the event is involving
     * @param time time at which event is created
     * @param server server that the WaitEvent belongs to
     */
    public WaitEvent(Customer customer, double time, Server server) {
        super(customer,time);
        this.server = server;
    }

    /**
     * Creates a ServedEvent to signal that the current customer can now be served.
     * Timestamp the current customer is being served is taken from the server.
     * @param servers Array of servers to be checked, not used in this case
     * @param gen RandomGenerator, not used in this case
     * @return ServedEvent
     */
    public ServedEvent getNextEvent(Server [] servers,RandomGenerator gen) {
        if (!this.server.canTakeWaitEvent()) {
            ServedEvent newEvent = new ServedEvent(this.getCustomer(), 
                this.server.getDoneTime(), this.server);
            this.server.setWaitEvent(newEvent);
            return newEvent;
        }
        return null;
    }

    /**
     * Modifies information in statistics if required.
     * @param statistics Not used in this case 
     */
    public void updateStatistics(Statistics statistics) {
        return;
    }

    /**
     * Formats the WaitEvent to print out its profile.
     */
    public String toString() {
        return (String.format("%.3f",this.getTime()) +
            ' ' + this.getCustomerID() + " waits to be served by " +
            server.getServerID());
    }

}

ServedEvent

package cs2030.simulator;


/**
 * ServedEvent class to simulate the start of service to a customer by a server.
 */
class ServedEvent extends Event {

    /** 
     * Server that the ServedEvent belongs to.
     */
    private Server server;

    /** 
     * Creates an ServedEvent.
     * @param customer customer that the event is involving
     * @param time time at which event is created
     * @param server server that the ServedEvent belongs to
     */

    public ServedEvent(Customer customer, double time, Server server) {
        super(customer,time);
        this.server = server;
    }
    /**
     * Creates a DoneEvent to signal that the service has been completed.
     * Time taken to complete the service is randomised by RandomGenerator.
     * DoneEvent is created at the new timestamp of current time of ServedEvent
     * added to the randomised service time.
     * @param servers Array of servers to be checked, not used in this case
     * @param gen RandomGenerator, to randomise service time
     * @return DoneEvent
     */

    public DoneEvent getNextEvent(Server [] servers,RandomGenerator gen) {
        double x = gen.genServiceTime​();
        DoneEvent newEvent = new DoneEvent(this.getCustomer(), 
            this.getTime() + x,this.server);
        this.server.setServedEvent(newEvent);
        return newEvent;
    }

    /**
     * Increases the customer served count in statistics.
     * and increase the waiting time by the customer if any
     * @param statistics statistics to be updated
     */
    public void updateStatistics(Statistics statistics) {
        statistics.increaseServed();
        statistics.increaseWaitingTime(this.getTime() - this.getCustomer().getTime());
    }

    /**
     * Formats the ServedEvent to print out its profile.
     */
    public String toString() {
        return (String.format("%.3f",this.getTime()) + ' ' +
            this.getCustomerID() + " served by " + server.getServerID());
    }
}

DoneEvent

package cs2030.simulator;

/**
 * DoneEvent class to simulate the completion of service to a customer by a server.
 */
class DoneEvent extends Event {
    /** 
     * Server that the DoneEvent belongs to.
     */
    private Server server;

    /** 
     * Creates an DoneEvent.
     * @param customer customer that the event is involving.
     * @param time time at which event is created.
     * @param server server that the DoneEvent belongs to.
     */
    public DoneEvent(Customer customer, double time, Server server) {
        super(customer,time);
        this.server = server;
    }
    /**
     * Creates a null object to signal no actual Event type is created.
     * Server is being updated with the service time the 
     * next event should adhere to if any.
     * @param servers Array of servers, not used in this case.
     * @param gen RandomGenerator, not used in this case.
     * @return null object.
     */

    public Event getNextEvent(Server [] servers,RandomGenerator gen) {
        this.server.flushDoneEvent();
        return null;
    }

    /**
     * Modifies information in statistics if required.
     * @param statistics Not used in this case 
     */
    public void updateStatistics(Statistics statistics) {
        return;
    }

    /**
     * Formats the DoneEvent to print out its profile.
     */
    public String toString() {
        return String.format("%.3f",this.getTime()) +
            ' ' + this.getCustomerID() + " done serving by " +
            server.getServerID();
    }

}

LeaveEvent

package cs2030.simulator;

/**
 * LeaveEvent class to simulate the act of customer leaving without being served.
 */
class LeaveEvent extends Event {
    private Server server;

    /** 
     * Creates an LeaveEvent.
     * @param customer customer that the event is involving.
     * @param time time at which event is created.
     */
    public LeaveEvent(Customer customer, double time) {
        super(customer,time);
    }

    /**
     * Creates a null object to signal no actual Event type is created.
     * @param servers Array of servers, not used in this case
     * @param gen RandomGenerator, not used in this case
     * @return null object
     */
    public Event getNextEvent(Server [] servers, RandomGenerator gen) {
        return null;
    }

    /**
     * Increases the customer leave count inside statistics.
     * @param statistics statistics to be updated
     */
    public void updateStatistics(Statistics statistics) {
        statistics.increaseLeft();
    }

    /**
     * Formats the LeaveEvent to print out its profile.
     */
    public String toString() {
        return String.format("%.3f",this.getTime()) +
            ' ' + this.getCustomerID() + " leaves";
    }

}

Test case: Input and Output style

1
1
5
1.0
1.0


0.000 1 arrives
0.000 1 served by 1
0.313 1 done serving by 1
0.314 2 arrives
0.314 2 served by 1
0.417 2 done serving by 1
1.205 3 arrives
1.205 3 served by 1
1.904 3 done serving by 1
2.776 4 arrives
2.776 4 served by 1
2.791 4 done serving by 1
3.877 5 arrives
3.877 5 served by 1
4.031 5 done serving by 1
[0.000 5 0]


1
2
10
1.0
1.0

0.000 1 arrives
0.000 1 served by 1
0.313 1 done serving by 1
0.314 2 arrives
0.314 2 served by 1
0.417 2 done serving by 1
1.205 3 arrives
1.205 3 served by 1
1.904 3 done serving by 1
2.776 4 arrives
2.776 4 served by 1
2.791 4 done serving by 1
3.877 5 arrives
3.877 5 served by 1
3.910 6 arrives
3.910 6 served by 2
3.922 6 done serving by 2
4.031 5 done serving by 1
9.006 7 arrives
9.006 7 served by 1
9.043 8 arrives
9.043 8 served by 2
9.105 9 arrives
9.105 9 waits to be served by 1
9.160 10 arrives
9.160 10 waits to be served by 2
10.484 7 done serving by 1
10.484 9 served by 1
10.781 9 done serving by 1
11.636 8 done serving by 2
11.636 10 served by 2
11.688 10 done serving by 2
[0.386 10 0]

Is there any improvement that I can make on my code? Some areas I can think of:

  • Coding style
  • Naming conventions
  • Design of proper interactions between objects (like what is passed as arguments and what is called)
  • OOP design principles
  • Any small recommendations on syntax/implementation of class methods/libraries to make my life slightly easier

In particular for the 3rd point, I have two dilemma(s):

  1. About the link between class responsibilities and what is the best way of invoking a method call between different objects in different scopes.

    EventManager is deemed to be in charge of holding a Statistics field, and holding a PriorityQueue of Events, since it's the Manager. In my implementation I could have written one of the following:

    A: newEvent.updateStatistics(statistics)

    B: statistics.updateStatistics(newEvent)

    In A, to make a method for the Event class, feels easier and more accessible in terms of code length. Each subclass of Event will have their own version of updating the statistics and hence avoid some dispatching on type of the different classes, which I believe is the principle of OOP if I'm not wrong on this.

    On the other hand, in B feels like its more fulfilling to responsibility. The object newEvent is the provider of the information, hence should be placed in the arguments.

    I chose A in the end as during the time I found B logical, I realised I have to do ugly dispatching and I am tempted to pass back statistics as arguments to Events, in the form of newEvent.f(this), in Statistics method updateStatistics. This just defeats my purpose of sending Events in as arguments from the start. What is your view on this dilemma of responsibility design vs the ease of coding to the programmer?

  2. I was kind of forced to put a property of Server into Event so that events could print the profile easily, but I was also forced to put a property of Event into Server, as I find that it would save me a lot of work to communicate to the WaitEvent in Server of what time to switch to when becoming a ServedEvent. Is this a form of cyclical dependency(a bad practice), and if it is, is it justifiable to do so?

\$\endgroup\$
  • \$\begingroup\$ It's very good that you've posted all you code but could you describe it a little bit more? \$\endgroup\$ – t3chb0t Sep 22 '18 at 7:55
  • \$\begingroup\$ Can anyone be kind enough to tell me how can I bump this post to make more people read this? \$\endgroup\$ – Prashin Jeevaganth Sep 23 '18 at 3:07
  • \$\begingroup\$ Just be more patient and wait a couple more days until the next week :-) there are always fewer readers on weekends. \$\endgroup\$ – t3chb0t Sep 23 '18 at 5:40
1
\$\begingroup\$

Naming & Type Embedded in Name

Avoid placing types in method names; it's not only redundant, but it forces you to change the name if the type changes.

class Server {
    // ..
    private int serverID;
class Customer {
    // ..
    private final int customerID;

When you see the class Server as a domain, it is semantically the same when you use id instead of serverID and the same for Customer

class Server {
    // ..
    private int id;

Data Encapsulation

Data encapsulation, sometimes referred to as data hiding, is the mechanism whereby the implementation details of a class are kept hidden from the user.

class EventManager {
    Server[] servers;    
    PriorityQueue<Event> events;    
    Statistics statistics = new Statistics();    
    RandomGenerator gen;

Currently EventManager reveals its implementation at package-private scope, but these members get never accessed and do not need to be visible outside of the class and should be private.

Preserve Whole Object

public EventManager(int numServers, int numCustomer,
                        int seed, double arrivalRate, double svcRate, double restRate) {
    // ..
    this.gen = new RandomGenerator(seed, arrivalRate, svcRate, restRate);
    // ..

Since the client already knows all variables of a RandomGenerator, he/she could directly pass in the object.

public EventManager(int numServers, 
                    int numCustomer,
                    RandomGenerator randomGenerator) {
    //..
    this.gen = randomGenerator;
    // ..

This has multiple benefits

Redundant initialization

class Server {

    // ..
    private Event served = null;

    private Event wait = null;

Initializing fields with null is redundant since the default value of a reference type is null. So the above is equivalent to

class Server {

    // ..
    private Event served;

    private Event wait;

Use of Static

class Server {
    private static int counter = 1;
    // ..

    public Server() {
        this.serverID = counter;
        counter++;
    }
class Customer {
    private static int counter = 1;

    public Customer(double time) {
        this.customerID = counter;
        this.time = time;
        counter++;
    }

A static variable is not bad at all but in this case it is against Data Encapsulation, because all instances of Server\ Customer share the static variable counter.

Imagine you involve these classes into some jUnit tests. If a test depends on a id, you will never know which id a customer has, because jUnit executes tests in unpredictable order. Tests should not depend on other tests, but as this stands, these tests would change the behavior of other ones.

Use a Factory

Create a Factory for Customer and Server that tracks the ids and passes them through the constructor.

Simple example:

class CustomerFactory {
    private int nextId;

    public CustomerFactory(int startId) {
        nextId = startId;
    }

    Customer createBy(double time) {
        return new Customer(time, nextId++);
    }
}

class Customer {
    private final int id;

    private final double time;

    Customer(double time, int id) { /* .. */ }
}

Tell, Don't Ask

Tell-Don't-Ask is a principle that helps people remember that object-orientation is about bundling data with the functions that operate on that data.

class EventComparator implements Comparator<Event> {

    public int compare(Event e1, Event e2) {
        if (e1.getTime() < e2.getTime()) {
            return -1;
        } else if (e1.getTime() > e2.getTime()) {
            return 1;
        } else if (e1.getCustomerID() < e2.getCustomerID()) {
            return -1;
        } else if (e1.getCustomerID() > e2.getCustomerID()) {
            return 1;
        } else {
            System.out.println("Bug with code, please check");
            return 0;
        }
    }
}

Instead of asking for time and customerID, tell e1 to operate with some data:

public int compare(Event e1, Event e2) {
    if (e1.isBefore(e2)) {
        return -1;
    } else if (e1.isAfter(e2)) {
        return 1;
    } else if (e1.hasSmallerIdThen(e2)) {
        return -1;
    } else if (e1.hasBiggerIdThen(e2)) {
        return 1;
    } else {
        System.out.println("Bug with code, please check");
        return 0;
    }
}

@Override

class Event {
    // ..

    public abstract Event getNextEvent(Server[] servers, RandomGenerator gen);

    public abstract void updateStatistics(Statistics statistics);

These methods get overridden by multiple classes and none of them uses @Override. It gives you as a programmer safety from the compiler's check that you really override a method and on the other hand it helps me as a reader to see at a glance which methods of the superclass get overridden.

@Override
public Event getNextEvent(Server[] servers, RandomGenerator gen) {

\$\endgroup\$
1
\$\begingroup\$

Anyway you MUST close the scanner after finishing using it just for saving the memory like this :

import java.util.Scanner;
import cs2030.`enter code here`simulator.Event;
import cs2030.simulator.Customer;
import cs2030.simulator.EventComparator;
import cs2030.simulator.EventManager;
import cs2030.simulator.ArrivalEvent;


//Main class to collect inputs from user required for the RandomGenerator, creates an EventManager to print the statistics for a sequence of events.
public class Main {
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        int seed = sc.nextInt();
        int numServers = sc.nextInt();
        int numCustomer = sc.nextInt();
        double arrivalRate = sc.nextDouble();
        double svcRate = sc.nextDouble();
        ////////////////////////////////////////////////
        sc.close();
        ////////////////////////////////////////////////
        double restRate = 1.0;

        EventManager eventManager = new EventManager(numServers, numCustomer,
            seed,arrivalRate,svcRate,restRate);
        eventManager.doService();
    }
}
\$\endgroup\$
  • \$\begingroup\$ alright thanks for the answer, will note for that. But design wise anything else? \$\endgroup\$ – Prashin Jeevaganth Sep 22 '18 at 14:21
1
\$\begingroup\$

You have written

else {
        System.out.println("Bug with code, please check");
        return 0;
    }

This means if two events have the same time and belong to the same customer, then you are printing that there is a bug. But this is wrong. Suppose a customer arrives and he is immediately served. Then ARRIVAL event and SERVED event have the same time stamp and belong to the same customer. I suggest that if two events have same time and same customer, then order them according to type of event. So, ARRIVAL event is first, followed by WAITS/LEAVES, followed by SERVED, followed by DONE

\$\endgroup\$
  • \$\begingroup\$ Hi, I think that is impossible, poll from the PriorityQueue removes the ArrivalEvent with Customer A from the priority queue, then we deduce that we have to create a new ServedEvent also involving A, then put back into the queue. There won't be a chance for 2 events involving the same customer to exist simultaneously in the same queue. Regarding type priority across different customers, it is also implicit that at the same timestamp, priority is given when customerID is lower, which happens since he arrived first, and became a DoneEvent earlier than the next arriving customer. \$\endgroup\$ – Prashin Jeevaganth Sep 23 '18 at 11:40
  • \$\begingroup\$ Is it passing all test cases? \$\endgroup\$ – Avinash Sep 23 '18 at 12:31
  • \$\begingroup\$ Yes, this code has passed all test cases when I submitted my code. Unless the test case is flawed, I am just submitting my code here to review on designing \$\endgroup\$ – Prashin Jeevaganth Sep 23 '18 at 12:47
  • \$\begingroup\$ Is there any reason to put rest rate as 1.0? Is this a part of requirement? Design is quite good. \$\endgroup\$ – Avinash Sep 23 '18 at 13:18
  • \$\begingroup\$ Hmm, it's part of the requirement, didn't really question why they set it to 1.0 \$\endgroup\$ – Prashin Jeevaganth Sep 23 '18 at 13:34

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.