Playing a game throughout the house

Target

I wrote this program in a few free hours when I was on an excursion as "youth leader". I had created a game to explore the house, with a 40-field mechanic, using dice for advancing and question-based setbacks.

The dice and questions were managed outside the program. De-facto this was just to show progressions of the different teams on a screen.

The names of the team-members have been changed to protect the innocent. I tried to keep the Game configurable aside from the teams.

That said, let's jump into the code:

Code

Main-class:

/**
 * Class for containing the main method. Will only start up the Screen and keep
 * running until screen terminates (or rather signals execution stop)
 * 
 * @author vogel612
 *
 */
public class Program {

    public static void main(String[] args) {
        Screen screen = new Screen();
        screen.startGame();
        while (screen.isRunning()) {
        }
    }
}

I doubt there's much to say here. I am slightly unhappy about the program exit construction, but I doubt there's much to change here...

Display

```
import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.FlowLayout;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.WindowEvent;
import java.util.Arrays;

import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingUtilities;
import javax.swing.border.LineBorder;

/**
* Screen Class: Contains... everything frankly. Teams their colors a few settings and handlers for buttons.
* While that maybe isn't the best idea, the simplicity of the problem allows this.
*
* @author vogel612
*
*/
public class Screen extends JFrame {

private static final long serialVersionUID = 8208211925302053925L;

private static final Color teamOneColor = new Color(0.3f, 0.0f, 0.

You can get rid of the busy-wait loop since Swing keeps another thread alive. It was hogging my cpu doing nothing.

You should have a good model/view separation from the start. When I do a small project like yours, I always start by doing a version where the GUI is just some output text to the console. But I am aware that I will move to a real GUI, so I modularize my code accordingly.

I think you are really abusing enum. It makes more sense to just have a normal Team class and then you can create as many Teams as you want.

Also I would not store the positions in the Team themselves, but in a Board class. When switching from a console based GUI to a Swing based GUI, all you have to do is changed the input method and the paint method. You should define some abstrac GUI interface with those two methods.

private static class Board {
    private final int fieldCount;
    private final LinkedHashMap teamsPositions = new LinkedHashMap<>();

    public Board(int fieldCount, List teams) {
        this.fieldCount = fieldCount;
        teams.forEach(team -> teamsPositions.put(team, 0));
    }

    // Should give more thought on the return value.  A team wins when they go over the max?
    public boolean move(Team team, int increment) {
        int oldPosition = teamsPositions.get(team);
        int position = oldPosition + increment;
        if (position >= fieldCount) {
            return false;
        } else {
            teamsPositions.put(team, position);
            return true;
        }
    }

    public int getFieldCount() {
        return fieldCount;
    }

    public LinkedHashMap getTeamsPositions() {
        return teamsPositions;
    }

}

private static class Team {
    private final Color teamColor;
    private final String teamName;

    Team(Color color, String name) {
        this.teamName = name;
        this.teamColor = color;
    }

    public Color getColor() {
        return teamColor;
    }

    public String getTeamName() {
        return teamName;
    }
}

On the functionality side, you should probably auto-toggle which team is active. But I don't know the rules and maybe sometimes some team skip their move.

An minor suggestion related to lambdas:

teams.stream().map(team -> 
        new JPanel() {
            private static final long serialVersionUID = -2035379018013186886L;
            {
                JLabel teamLabel = new JLabel(team + ": " + team.getTeamName());
                setBackground(team.getColor());
                add(teamLabel);
            }
        })
        .forEach(legendPanel -> Screen.this.add(legendPanel));

But it would actually be cleaner to write a static method to create a JPanel from a Team (not just for the lambda) and then the streaming simplifies to:

teams.stream()
        .map(Screen::createTeamLegendPanel)
        .forEach(legendPanel -> Screen.this.add(legendPanel));

StackExchange Code Review Q#68616, answer score: 6