Discrete event simulation of a prioritized lunch queue

Note

This post is a continuation of Discrete event simulation of a prioritized lunch queue in Java (Data structures). Please refer to it for problem description.

This part about "algorithms": all classes that are more about doing rather than representing information.

PrioritizedQueue.java:

package net.coderodde.simulation.lunch;

import java.util.ArrayDeque;
import java.util.EnumMap;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Queue;

/**
 * This class implements a FIFO queue over priority categories. Not to be 
 * confused with a priority queue.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Dec 3, 2015)
 */
final class PrioritizedQueue {

    private final Map> map 
            = new EnumMap<>(AcademicDegree.class);

    private int size;

    void push(LunchQueueEvent event) {
        AcademicDegree degree = event.getPerson().getAcademicDegree();
        map.putIfAbsent(degree, new ArrayDeque<>());
        map.get(degree).add(event);
        ++size;
    }

    boolean isEmpty() {
        return size == 0;
    }

    LunchQueueEvent pop() {
        if (isEmpty()) {
            throw new NoSuchElementException(
                    "Popping from an empty prioritized queue.");
        }

        for (Queue queue : map.values()) {
            if (!queue.isEmpty()) {
                --size;
                return queue.remove();
            }
        }

        throw new IllegalStateException(
                "This should never happend. Please debug.");
    }
}

RandomPopulationGenerator.java:

```
package net.coderodde.simulation.lunch;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Random;
import static net.coderodde.simulation.lunch.Utils.checkMean;
import static net.coderodde.simulation.lunch.Utils.checkStandardDeviation;

/**
* This class facilitates random generation of population.

This is an interesting programming exercise. I appreciate that you made a significant attempt to make your code elegant, but it didn't work out that well, in my opinion.

I'm not a fan of the Niklaus Wirth quote in your related question ("Algorithms + Data Structures = Programs"). Data structures should be designed to support your algorithms, and there is opportunity for the sum to be greater than the parts, if they work together well. (Example: the Union-Find data structure, which is the heart of the associated algorithm.) This synergy is especially important for an object-oriented language like Java, in which the goal is to design classes that act like "smart data".

Anyway, I find it hard to split the review for that reason, and will just answer everything here.

As for the code length… I've written a solution in ~400 lines, about a third of your solution of ~1200 lines. Granted, about 90 of your lines were dedicated to naming your model customers (Why bother?), but still, that's a substantial simplification.

Fluent interface

A lot of the complexity in the code goes to support the fluent configuration calls used in Demo.main(). It feels like you are trying to mold Java into Objective C. I think it's a bad idea because

-
These clauses tend to introduce different scopes. For example, the with() clause needs to be followed by a peopleWithDegree() call. It's not obvious what the correct "grammar" of the "sentence" has to be.

In contrast, chained calls work very well for jQuery since every call produces a jQuery object just like any other jQuery object.

It would help a bit if you toned down the interface by "uncurrying" the calls. If you had…

withNPeopleWithDegree(15, AcademicDegree.DOCTOR)

… then at least you wouldn't be introducing another grammatical context ("selectors", as you call them in your code).

-
The grammar is somewhat arbitrary, and adherence to the grammar makes your code rigid. Who would guess withCashier(…) is the special clause at the end of the "sentence" that kicks off the simulation (in Simulator.CashierSelector.withCashier(…))?

Even more surprising: who would guess that Simulator.simulate() doesn't kick off a simulation at all, but merely returns a PopulationSelector? (Note that the public Simulator.simulate() is completely different from the private Simulator.simulate(Population, Cashier), which is weird.)

Note that your grammar is inspired by English word order, which isn't necessarily the most logical word order. A Subject-Object-Verb language would have avoided this specific problem with simulate().

-
It's still a lot of code for little benefit. It's debatable whether it makes your Demo.main() more readable, but you definitely pay the price in the clutter you introduce to RandomPopulationGenerator.

If you really wanted an elegant way to configure the simulation parameters, why not just implement a configuration file format, perhaps based on xml or yaml?

The Simulator and Demo classes are tightly coupled to each other. Demo is nothing more than a main() method to launch Simulator with specific parameters. I suggest merging the two classes.

Models

I think that you are modelling the problem too literally. You would be better off naming your classes using standard vocabulary from queuing theory, rather than the objects specific to this scenario. The code would be more reusable, and easier to understand by other programmers. For example,

• Cashier is a "server"

• Person is a "customer"

• AcademicDegree is a "customer class"

Note that you can still name instances using scenario-specific words (e.g. Server cashier = new Server(…)).

You modelled the queue discipline as a single PrioritizedQueue. When certain customers can queue-jump, I'm not sure that it's fair to call it a "queue" anymore. I'd call it four queues. (That's how the priority scheme would be implemented in real life, right?) With four queues, you can eliminate the awkward PrioritizedQueue class that is, in your own words, "not to be confused with a priority queue".

Accounting

The accounting is complicated by the fact that the statistics are not stored with the associated objects. Rather, you have a SimulationResult that is calculated in Simulator.postprocess() from the arrivalEventMap and the servedEventMap. (You also have an inexplicable special case in Simulator.simulate(Population, Cashier) for zero-sized populations.)

The presence of helper methods in Simulator — preprocess(), initWaitingTimeStructures(), precomputeWaitingTimes(), precomputeDeviationsPhase1(), precomputeDeviationsPhase2(), computeStandardDeviations(), loadStatistics(), computeCashierStatistics(), and postprocess() — is a code smell. The fact that it takes so many steps, which probably need to be executed in a specific order, indicates that the code is fragile. If you have a good OOP design, no pre- and post-processing shou

StackExchange Code Review Q#113607, answer score: 2