Skip to main content
added 72 characters in body
Source Link
Reinderien
  • 71.2k
  • 5
  • 76
  • 257

It's not kmh (kilometre hours), it's km/h (kilometres per hour).

It's not kmh (kilometre hours), it's km/h (kilometres per hour).

Source Link
Reinderien
  • 71.2k
  • 5
  • 76
  • 257

Your Light class, being a data model, should not care about colours, and only semantic states in an enum (i.e. stopping, stopping soon, or running). Prefer this over a colour or a colour index.

Do not track a Point in your light: the simulation model only cares about a horizontal offset, and the vertical offset is a rendering detail that should be pulled out to your GUI.

Get rid of your Light.setSeconds. Good encapsulation dictates that the light should know how to update its own time quantity.

This structure:

        if (colorIndex == 0) {
            // red get 15 seconds
            seconds = 15;
        }
        else if (colorIndex == 1) {
            seconds = 10;//green light
        } else {
            seconds = 3;//yellow light
        }

is better-represented with an expression-valued switch. Also notice that you've repeated that logic; don't do that.

Delete @SuppressWarnings("serial").

Your Simulator class needs to be split in two: a Simulator logic class with no GUI elements, and a SimulatorFrame class with only GUI elements.

Nearly all of your class members need to be made private final.

Move your button listeners from lambdas to class members since they're sufficiently large.

startSimulation exhibits a reuse antipattern. Rather than clearing and reinitializing lists in-place, you should be discarding and re-constructing a simulator object.

Don't rand.nextInt(2) + 3;; call the overload that has a lower and upper bound.

Add constant variables for magic numbers like 350.

Your invokeLater is not necessary.

I have not demonstrated this, but getLight needs to be refactored to use a binary search.

if (seconds > 0) is better written as if (seconds >= 1), since that's what you actually care about: being able to decrement without going negative. In the former implementation, if you switch to floating-point times you will get some nasty surprises.

light will never be null; I'm not sure why you check for that.

Don't have isStopped return a boxed Boolean; instead return boolean.

Car being its own thread is not a good idea; it should tick as a call from the main simulation timer. Otherwise, your pause is broken and only hides rendering but does not pause the car movement internally.

Your rendering uses too many magic numbers, and fails to prevent text overlap. Use font metrics to calculate and avoid these overlaps. The method I show is very inefficient but is simple and does work.

Suggested

SimulatorFrame.java

package traffic;

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;


public class SimulatorFrame extends JFrame {
    private enum State { stopped, paused, running }
    private State state = State.stopped;

    private final JButton btnStart = new JButton("Start");
    private final JButton btnPause = new JButton("Pause");

    private final Map<Light.State, Color> stateColors = Map.of(
        Light.State.started, Color.green,
        Light.State.stopSoon, Color.yellow,
        Light.State.stopped, Color.red
    );

    private final Timer t = new Timer(100, this::clockTicks);
    private Simulator sim;

    SimulatorFrame() {
        btnPause.setEnabled(false);
        btnPause.addActionListener(this::pauseClicked);
        btnStart.addActionListener(this::startClicked);

        JPanel jp = new JPanel();
        jp.setLayout(new BorderLayout());
        JPanel south = new JPanel();
        south.setLayout(new FlowLayout());
        south.add(btnStart);
        south.add(btnPause);
        jp.add(south, BorderLayout.SOUTH);
        jp.add(new Road(), BorderLayout.CENTER);
        getContentPane().add(jp);

        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setSize(1200, 700);
        setVisible(true);
    }

    private void pauseClicked(ActionEvent event) {
        state = switch (state) {
            case running -> {
                btnPause.setText("Continue");
                t.stop();
                yield State.paused;
            }
            case paused -> {
                btnPause.setText("Pause");
                t.start();
                yield State.running;
            }
            default -> throw new IllegalStateException();
        };
    }

    private void startClicked(ActionEvent event) {
        state = switch (state) {
            case stopped -> {
                btnStart.setText("Stop");
                btnPause.setEnabled(true);
                sim = new Simulator();
                t.start();
                yield State.running;
            }
            case running, paused -> {
                btnStart.setText("Start");
                btnPause.setText("Pause");
                btnPause.setEnabled(false);
                t.stop();
                sim = null;
                yield State.stopped;
            }
        };
    }

    private void clockTicks(ActionEvent event) {
        sim.tick();
        getContentPane().repaint();
    }

    private class Road extends JPanel {
        private record TextExtent(
            int y, int x1, int x2
        ) { }

        private static final int
            LIGHT_RADIUS = 20,
            LIGHT_DIAMETER = 2*LIGHT_RADIUS,
            LIGHT_Y = 350,
            LIGHT_STICK_HEIGHT = 60,
            CAR_Y = 400,
            CAR_TEXT_Y = 470;

        @Override
        protected void paintComponent(Graphics g) {
            super.paintComponent(g);
            if (sim == null) return;

            for (Light light: sim.getLights())
                paintLight(g, light);

            FontMetrics metrics = g.getFontMetrics();
            List<TextExtent> lines = new ArrayList<>();

            for (Car car: sim.getCars())
                paintCar(g, metrics, car, lines);
        }

        private void paintLight(Graphics g, Light light) {
            g.setColor(Color.BLACK);
            g.drawString(Integer.toString(light.getSeconds()), light.x, LIGHT_Y - 60);
            Color c = stateColors.get(light.getState());
            g.setColor(c);
            g.fillOval(light.x, LIGHT_Y, LIGHT_DIAMETER, LIGHT_DIAMETER);
            g.drawLine(
                light.x + LIGHT_RADIUS, LIGHT_Y + LIGHT_RADIUS,
                light.x + LIGHT_RADIUS, LIGHT_Y + LIGHT_STICK_HEIGHT
            );
        }

        private void paintCar(Graphics g, FontMetrics metrics, Car car, List<TextExtent> lines) {
            Color c = car.getColor();
            g.setColor(c);
            int x = (int)car.getX();
            g.fillRect(x, CAR_Y, 20, 20);

            String desc = "(%.0f, 0) %.2f km/h".formatted(
                car.getX(), car.getVelocity()
            );
            int w = metrics.stringWidth(desc),
                x2 = x + w,
                y;

            for (y = 0;; y++) {
                boolean found = false;
                for (TextExtent text: lines) {
                    if (
                        y == text.y
                            && x < text.x2
                            && x2 > text.x1
                    ) {
                        found = true;
                        break;
                    }
                }
                if (!found) break;
            }

            lines.add(new TextExtent(y, x, x2));

            g.setColor(Color.black);
            g.drawString(
                desc, x,
                CAR_TEXT_Y + metrics.getHeight()*y
            );
        }
    }

    public static void main(String[] args) {
        new SimulatorFrame();
    }
}

Simulator.java

package traffic;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Random;


public class Simulator {
    private final ArrayList<Light> lights = new ArrayList<>();
    private final ArrayList<Car> cars = new ArrayList<>();
    private final Random rand = new Random();
    private int ticks = 0;

    public Simulator() {
        int nLights = rand.nextInt(3, 5),
            distance = 1000 / nLights;

        for (int i = 0; i < nLights; i++) {
            int x = (i + 1) * distance;
            Light l = new Light(x, Light.State.getRandom(rand));
            lights.add(l);
        }

        for (int i = 0; i < 3; i++) {
            int x = rand.nextInt(500);
            cars.add(new Car(x, rand));
        }
    }

    public List<Light> getLights() {
        return Collections.unmodifiableList(lights);
    }

    public List<Car> getCars() {
        return Collections.unmodifiableList(cars);
    }

    private Light getLight(double x) {
        for (Light light: lights) {
            double diff = light.x - x;
            if (diff < 5 && diff > -1)
                return light;
        }
        return null;
    }

    public void tick() {
        boolean newCar = rand.nextInt(100) < 1;
        if (newCar) {
            int x = 50;
            cars.add(new Car(x, rand));
        }

        List<Car> toRemove = new ArrayList<>();
        for (Car car: cars) {
            double x = car.getX();
            Light light = getLight(x);
            car.setLight(light);
            car.tick();
            if (car.expired())
                toRemove.add(car);
        }
        for (Car car: toRemove)
            cars.remove(car);

        ticks++;
        if (ticks % 10 == 0) {
            for (Light light: lights)
                light.tick();
        }
    }
}

Light.java

package traffic;

import java.util.Random;

public class Light {
    public enum State {
        started, stopSoon, stopped;

        public State getNext() {
            int i = (ordinal() + 1) % values().length;
            return values()[i];
        }

        public static State getRandom(Random rand) {
            int i = rand.nextInt(values().length);
            return values()[i];
        }
    }

    private State state;
    private int seconds;
    public final int x;

    public Light(int x, State initialState) {
        this.x = x;
        setState(initialState);
    }

    public void tick() {
        if (seconds >= 1)
            seconds -= 1;
        else setState(state.getNext());
    }

    public int getSeconds() {
        return seconds;
    }

    public void setState(State state) {
        this.state = state;

        seconds = switch (state) {
            case stopped -> 15;
            case started -> 10;
            default -> 3;
        };
    }

    public State getState() {
        return state;
    }

}

Car.java

package traffic;

import java.awt.Color;
import java.util.Random;


public class Car {
    private static final Color[] colors = {
        Color.blue, Color.lightGray, Color.cyan, Color.magenta
    };
    private final double velocity;
    private final Color color;
    private double x;
    private Light light;

    public Car(double x, double velocity, Color color) {
        this.x = x;
        this.velocity = velocity;
        this.color = color;
    }

    public Car(double x, Random rand) {
        this(x, randomVelocity(rand), randomColor(rand));
    }

    private static double randomVelocity(Random rand) {
        return rand.nextDouble(0, 25);
    }

    private static Color randomColor(Random rand) {
        return colors[rand.nextInt(colors.length)];
    }

    public double getX() {
        return x;
    }

    public double getVelocity() {
        if (!isStopped()) {
            return velocity;
        }
        return 0;
    }

    public Color getColor() {
        return color;
    }

    private boolean isStopped() {
        return light != null && light.getState() == Light.State.stopped;
    }

    public void tick() {
        x += getVelocity() * 0.1;
    }

    public void setLight(Light light) {
        this.light = light;
    }

    public boolean expired() {
        return x >= 1200;
    }
}

Output

simulation output