Optimizations and comments

- Made the code more function-oriented (less mutable state)
- Added a few comments
- Moved action handling to single method
- Usage of Collections.shuffle() for better code readability.

src/main/java/eu/tankernn/julklapp/Julspel.java

@@ -4,10 +4,9 @@ import java.awt.EventQueue;
 import java.awt.GridLayout;
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
-import java.awt.event.ItemEvent;
-import java.awt.event.ItemListener;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -20,7 +19,7 @@ import javax.swing.JComboBox;
 import javax.swing.JFrame;
 import javax.swing.JOptionPane;
 
-public class Julspel {
+public class Julspel implements ActionListener {
 	public static final String[] NAMES = { "Claes", "Lars", "Fredrik", "Rune-Bertil", "Pepita", "Magnus", "Kim",
 			"Frans", "Erik" },
 			GIFTS = { "Nalle", "PS4", "PC", "Spel", "Lego", "Pengar", "Biobiljett", "Headset", "Mus", "Tangentbord",
@@ -32,8 +31,6 @@ public class Julspel {
 	private JComboBox<Difficulty> difficultyBox = new JComboBox<Difficulty>(Difficulty.values());
 	private JButton showAllBtn = new JButton("Visa namn och klappar."), startBtn = new JButton("Starta spelet!");
 
-	private int difficulty = 3;
-
 	/**
 	 * Launch the application.
 	 */
@@ -61,46 +58,44 @@ public class Julspel {
 	 * Initialize the contents of the frame.
 	 */
 	private void initialize() {
-
 		frame = new JFrame();
 		frame.setLayout(new GridLayout(3, 1));
-
-		difficultyBox.addItemListener(new ItemListener() {
-			@Override
-			public void itemStateChanged(ItemEvent e) {
-				difficulty = ((Difficulty) e.getItem()).pairs;
-			}
-		});
+		
 		frame.add(difficultyBox);
-		showAllBtn.addActionListener(new ActionListener() {
-			@Override
-			public void actionPerformed(ActionEvent e) {
-				displayLists(NAMES, GIFTS);
-			}
-		});
+		showAllBtn.addActionListener(this);
 		frame.add(showAllBtn);
-		startBtn.addActionListener(new ActionListener() {
-			@Override
-			public void actionPerformed(ActionEvent e) {
-				startGame();
-			}
-		});
+		startBtn.addActionListener(this);
 		frame.add(startBtn);
 
 		frame.pack();
 		frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 	}
 
-	private void startGame() {
-		Map<String, String> associations = generateAssociations();
+	@Override
+	public void actionPerformed(ActionEvent e) {
+		Object src = e.getSource();
+		if (src.equals(showAllBtn))
+			displayLists(NAMES, GIFTS);
+		else if (src.equals(startBtn))
+			startGame(generateAssociations(NAMES, GIFTS, ((Difficulty) difficultyBox.getSelectedItem()).pairs));
+	}
+	
+	/**
+	 * Start and play the game.
+	 */
+	private void startGame(Map<String, String> associations) {
 		Iterator<Entry<String, String>> entries = associations.entrySet().iterator();
 
+		// Iterate through all but the last entry
 		for (int i = 0; i < associations.size() - 1; i++) {
 			Entry<String, String> e = entries.next();
 			JOptionPane.showMessageDialog(frame, e.getKey() + " får " + e.getValue() + ".");
 		}
 
+		// Get the last entry
 		Entry<String, String> secret = entries.next();
+
+		// Prompt user
 		String guessName = JOptionPane.showInputDialog(frame, "Vem är kvar?").trim().toLowerCase();
 		String guessGift = JOptionPane.showInputDialog(frame, "Vad får " + guessName + "?").trim().toLowerCase();
 
@@ -120,18 +115,29 @@ public class Julspel {
 
 		JOptionPane.showMessageDialog(frame, response);
 	}
+	
+	/**
+	 * Generate a map of name -> gift pairs.
+	 * @return The generated map
+	 */
+	private Map<String, String> generateAssociations(String[] nameArr, String[] giftArr, int difficulty) {
+		List<String> names = Arrays.asList(nameArr), gifts = Arrays.asList(giftArr);
 
-	private Map<String, String> generateAssociations() {
-		Map<String, String> assoc = new HashMap<String, String>();
-		List<String> names = Arrays.asList(NAMES), gifts = Arrays.asList(GIFTS);
-
+		// Shrink to match difficulty
 		names = randomFilter(names, difficulty);
 		gifts = randomFilter(gifts, difficulty);
+
+		// Show lists to user
 		displayLists(names.toArray(new String[names.size()]), gifts.toArray(new String[names.size()]));
 
-		while (!names.isEmpty()) {
-			assoc.put(names.remove(rand.nextInt(names.size())), gifts.remove(rand.nextInt(gifts.size())));
-		}
+		Collections.shuffle(names, rand);
+		Collections.shuffle(gifts, rand);
+
+		// Fill the hashmap
+		Map<String, String> assoc = new HashMap<String, String>();
+		for (int i = 0; i < names.size(); i++)
+			assoc.put(names.get(i), gifts.get(i));
+
 		return assoc;
 	}
 
@@ -146,30 +152,30 @@ public class Julspel {
 	 */
 	private List<String> randomFilter(List<String> list, int newSize) {
 		ArrayList<String> newList = new ArrayList<String>(list);
-		while (newList.size() > newSize) {
+		while (newList.size() > newSize)
 			newList.remove(rand.nextInt(newList.size()));
-		}
 		return newList;
 	}
 
 	private void displayLists(String[] names, String[] gifts) {
 		StringBuilder sb = new StringBuilder();
 		sb.append("<html>");
-		sb.append("<h3>Namn:</h3><ul>");
-
-		for (String name : names) {
-			sb.append("<li>").append(name).append("</li>");
-		}
+		sb.append(listWithHeader("Names:", names));
+		sb.append(listWithHeader("Gifts:", gifts));
+		sb.append("</html>");
+		JOptionPane.showMessageDialog(frame, sb);
+	}
 
-		sb.append("</ul><h3>Klappar:</h3><ul>");
+	private String listWithHeader(String header, String[] list) {
+		StringBuilder sb = new StringBuilder();
+		sb.append("<h3>").append(header).append("</h3><ul>");
 
-		for (String gift : gifts) {
-			sb.append("<li>").append(gift).append("</li>");
+		for (String str : list) {
+			sb.append("<li>").append(str).append("</li>");
 		}
 
-		sb.append("</ul></html>");
-
-		JOptionPane.showMessageDialog(frame, sb);
+		sb.append("</ul>");
+		return sb.toString();
 	}
 
 }