Swing and multi threading: why are the cars colliding?

Question

This is the implementation I made for cross-road concurrent app with a graphical user interface but there's a collision somehow after running the app for a while and can't really get my head around the actual problem please forgive the poor reduction I'm new here and I appreciate any support

My Window class:

package com.example.trafic;

import javax.swing.*;

public class Window extends JFrame {
    View view = new View();

    public Window(){
        this.setTitle("Trafic App");
        this.setSize(720, 450);
        this.setLocationRelativeTo(null);
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.setContentPane(view);
        this.setResizable(false);
        this.setVisible(true);
    }
}

my View class:

package com.example.trafic;

import javax.swing.*;
import java.awt.*;

public class View extends JPanel {
    //private Thread t;

    // DIMENSION
    public int width = 720;
    public int height = 420;
    public int route_width = 100;
    public int lamp_width = 30;
    public int car_width = 60;
    public int car_height = 70;

    public int positionV = 0;
    public int positionH = 0;
    public int feu = 1;

    @Override
    public void paintComponent(Graphics g2) {
        // TEXTTURE
        g2.setColor(Color.DARK_GRAY);
        g2.fillRect(0, 0, width, height);

        // ROUTE
        g2.setColor(Color.LIGHT_GRAY);
        g2.fillRect( width / 2 - route_width / 2, 0, route_width, 420);
        g2.fillRect(0, height / 2 - route_width / 2, 720, route_width);

        // LAMPE
        g2.setColor((feu == 1 ) ? Color.RED : Color.GREEN );
        g2.fillOval(width / 2 - route_width, height / 2 - route_width, lamp_width, lamp_width);
        g2.setColor((feu == 2 ) ? Color.RED : Color.GREEN);
        g2.fillOval(width / 2 + route_width - 30, height / 2 - route_width , lamp_width, lamp_width);
        ImageIcon ia = new ImageIcon(View.class.getResource("/images/carh.png"));
        Image carH = ia.getImage();
        ImageIcon ib = new ImageIcon(View.class.getResource("/images/carv.png"));
        Image carV = ib.getImage();
        g2.drawImage(carH, 10 + width / 2 - car_width / 2, 10 + positionH, this);
        g2.drawImage(carV, 720 - positionV, 10 + height / 2 - car_height / 2, this);
    }

    public void setLight(int feu){
        this.feu = feu;
        this.repaint();
    }

    public void setCarV(int i){
        this.positionV = i;
        this.repaint();
    }

    public void setCarH(int i) {
        this.positionH = i;
        this.repaint();
    }
}

my Task class:

package com.example.trafic;

public class Task implements Runnable {
    Window frame;
    static int Feu = 1;

    public Task(Window frame) {
        this.frame = frame;
    }

    public void run() {
        while (true) {
            try {
                Thread.sleep(4000);
                if (Feu == 1) {
                    Feu = 2;
                    System.out.println("Feu 2 ON");
                } else {
                    Feu = 1;
                    System.out.println("Feu 1 ON");
                }
                frame.view.setLight(Feu);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

my Run class:

package com.example.trafic;

public class Run implements Runnable {
    View view;

    public Run(View view) {
        this.view = view;
    }

    public void run() {
        while (Task.Feu == 2) {
            try {
                this.wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }

        while(true){
            int i = 0 ;

            int l = (Task.Feu == 1) ? 720 : 450;
            for (int j = 0; j < l; j++) {
                try {
                    Thread.sleep(2);
                    if ((Task.Feu == 1)) {
                        view.setCarV(++i);
                    } else {
                        view.setCarH(++i);
                    }
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }
    }
}

my App main class:

package com.example.trafic;

public class App {
    public static void main(String[] args) throws InterruptedException {
        Window frame = new Window();
        new Thread(new Task(frame)).start();
        new Thread(new Run(frame.view)).start();
    }
}

you'll have to be more specific about where in your code it goes wrong, and what exactly. — Stultuske, Jan 29 '19 at 10:54
1) For better help sooner, [edit] to add a [MCVE] or [Short, Self Contained, Correct Example](http://www.sscce.org/). 2) Please use code formatting for code and code snippets, structured documents like HTML/XML or input/output. To do that, select the text and click the `{}` button at the top of the message posting/editing form. — Andrew Thompson, Jan 29 '19 at 11:13
It seems that the problem is that when you change lights you do not check the the other car is not in the junction. For example when light changes to green for carh, carh should not move before carv exited the junction. — c0der, Jan 29 '19 at 12:52
By stopping the car on red only if it before the traffic light. For more help post mcve. — c0der, Jan 29 '19 at 13:45

score 1 · Answer 1 · answered Jan 29 '19 at 19:23

The following code implements Model-View-Controller pattern to demonstrate a simple junction control, that was attempted in the question.
The code is one-file MCVE: it can be copy pasted into Junction.java and run.
Please note the comments:

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.Image;
import java.net.MalformedURLException;
import java.net.URL;
import javax.swing.ImageIcon;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.Timer;

//serves as controller. "wires" up model, view and updating thread
public class Junction {

    private static final int TRAFFIC_LIGHT_CYCLE = 4000;
    private boolean isVerticalRed = false;

    Junction() {
        Model model = new Model();
        Window frame = new Window(model);
        model.setObserver(frame);
        //use time to change lights
        Timer timer = new Timer(TRAFFIC_LIGHT_CYCLE, e->{
            isVerticalRed = ! isVerticalRed;
            model.setVerticalRed(isVerticalRed);
        });

        timer.start();
        new Thread(new CarsUpdater(model)).start();
    }

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

//view of the mvc model. listens to changes in model using Observer interface
class Window extends JFrame implements Observer{

    private final View view;

    public Window(Model model){

        this.setTitle("Trafic App");
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        view = new View(model);
        this.setContentPane(view);
        pack();
        this.setResizable(false);
        this.setVisible(true);
    }

    //respond to model changes
    @Override
    public void changed() {
        repaint();
    }
}

class View extends JPanel {

    // DIMENSION
    private final static int ROUTE_WIDTH = 100, LAMP_WIDTH = 30;
    private final int CAR_WIDTH = 60, CAR_HEIGHT = 70;

    //Construct resources once
    private Image carH, carV;

    private final Model model;

    View(Model model) {

        this.model = model;

        //Construct resources once
        try {
            //use publically available resources to make code mcve 
            ImageIcon ia = new ImageIcon(new URL("https://i.imgur.com/wIO1liB.png"));
            carH = ia.getImage();
            ImageIcon ib = new ImageIcon(new URL("https://i.imgur.com/gEbZMHe.png"));
            carV = ib.getImage();
        } catch (MalformedURLException ex) {
            ex.printStackTrace();
        }

        setPreferredSize(new Dimension(model.getWidth(), model.getHeight()));
    }

    @Override
    public void paintComponent(Graphics g2) {

        super.paintComponent(g2);
        // TEXTTURE
        g2.setColor(Color.DARK_GRAY);
        g2.fillRect(0, 0, model.getWidth(), model.getHeight());

        // ROUTE
        g2.setColor(Color.LIGHT_GRAY);
        g2.fillRect( model.getWidth() / 2 - ROUTE_WIDTH / 2, 0, ROUTE_WIDTH, 420);
        g2.fillRect(0, model.getHeight() / 2 - ROUTE_WIDTH / 2,  model.getWidth(), ROUTE_WIDTH);

        // LAMPE
        g2.setColor(model.isVerticalRed() ?  Color.RED : Color.GREEN  );
        g2.fillOval(model.getWidth() / 2 - ROUTE_WIDTH,  model.getHeight() / 2 - ROUTE_WIDTH, LAMP_WIDTH, LAMP_WIDTH);

        g2.setColor(model.isVerticalRed() ? Color.GREEN : Color.RED);
        g2.fillOval( model.getWidth() / 2 + ROUTE_WIDTH - 30,  model.getHeight()/ 2 - ROUTE_WIDTH , LAMP_WIDTH, LAMP_WIDTH);

        g2.drawImage(carH, 10 + model.getWidth() / 2 - CAR_WIDTH / 2, 10 + model.getVerticalCarPosition(), this);
        g2.drawImage(carV,  model.getWidth() - model.getHorizontalCarPosition(), 10 +  model.getHeight() / 2 - CAR_HEIGHT / 2, this);
    }
}

interface Observer {
    void changed();
}

//model encapsulates information that view needs
class Model {

    private final static int W = 720, H = 420;
    private static final long SPEED = 2;
    private Observer observer;

    //is read / write by more than one thread getters and setters
    //need to be synchronized
    private boolean isVerticalRed = false;     //boolean to flag which light is red
    private int hPosition = 0,  vPosition = 0;

    boolean isVerticalRed() {
        return isVerticalRed;
    }

    void setVerticalRed(boolean isVerticalRed) {
        this.isVerticalRed = isVerticalRed;
        notifyObserver();
    }

    int getHorizontalCarPosition(){
        return hPosition;
    }

    //move horizontal car continuously, until meets stop condition
    void clearHorizontalCarFromJunction() throws InterruptedException{
        while(hPosition != W / 3){
            Thread.sleep(SPEED);
            stepHorizontal();
        }
    }

    //move horizontal one step
    void stepHorizontal(){
        int newH= hPosition  +1 > W ?  0 : hPosition +1;
        setHorizontalCarPosition(newH);
    }

    void setHorizontalCarPosition(int i){
        hPosition = i;
        notifyObserver();
    }

    //move vertical car continuously, until meets stop condition
    void clearVerticalCarFromJunction() throws InterruptedException{
        while(vPosition != H / 4){
            Thread.sleep(SPEED);
            stepVertical();
        }
    }
    //move vertical car one step
    void stepVertical(){
        int newV = vPosition +1 > H ?  0 : vPosition +1;
        setVerticalCarPosition(newV);
    }

    int getVerticalCarPosition(){
        return vPosition;
    }

    void setVerticalCarPosition(int i) {
        vPosition = i;
        notifyObserver();
    }

    int getWidth(){ return W;   }

    int getHeight(){ return H;  }

    void setObserver(Observer observer) {
        this.observer = observer;
    }

    void notifyObserver(){
        if(observer != null) {
            observer.changed();
        }
    }

    long getSpeed(){
        return SPEED;
    }
}

class CarsUpdater implements Runnable {

    private final Model model;

    public CarsUpdater(Model model) {
        this.model = model;
    }

    @Override
    public void run() {

        while(true){;

            try {

                Thread.sleep(model.getSpeed());
                if (model.isVerticalRed()) {
                    model.clearVerticalCarFromJunction();
                    model.stepHorizontal();
                } else {
                    model.clearHorizontalCarFromJunction();
                    model.stepVertical();
                }

            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

Thank you very much sir, you'll save me in my mini project i'll never forget your aid — Toufik Cech, Jan 29 '19 at 20:20

Swing and multi threading: why are the cars colliding?

1 Answers1