patternjavaModerate
"Lights off" puzzle
Viewed 0 times
lightspuzzleoff
Problem
Lights Off is a puzzle game consisting of an \$n\times n\$ grid of lights. At the beginning of the game, some of the lights are switched on. When a light is activated, it and its four neighbors in the cardinal directions are toggled. The objective is to turn off all the lights.
Input:
Output should be
by selecting cells \$(0,0)\$, \$(1,0)\$, \$(1,1)\$, \$(2,1)\$, and \$(2,2)\$
I'd like to know if my code can be made any more efficient.
```
import javax.swing.*;
import java.awt.*;
import javax.swing.JFrame;
import java.util.Scanner;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
public class lightOff extends JFrame implements ActionListener
{
public static final int W = 400;
public static final int H = 200;
JButton[][] lights = new JButton[3][3];
int COLS = 3, ROWS = 3;
public lightOff()
{
super("Light Off");
setSize(W,H);
setLayout(new GridLayout(3,3));
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
Scanner kb = new Scanner(System.in);
String[][] input = {{"0","0","0"},{"1","1","0"},{"0","1","0"}};
for(int i=0;i<3;i++)
{
for(int j=0;j<3;j++)
{
lights[i][j] = new JButton(); // Initializing all JButton
lights[i][j].addActionListener(this); // Registering listener
lights[i][j].setText(input[i][j]); // setting text of each
// button as per input String
add(lights[i][j]); // adding JButton to the JFrame
}
}
}
public static void main(String[] args)
{
lightOff obj1 = new lightOff();
obj1.setVisible(true);
}
public void actionPerformed(ActionEvent e)
{
JButton action = (JButton)e.getSource();
if(action==lights[0][0])
{
if("0".equals(lights[0][0].getText()))
lights[0][0].setText("1");
else
lights[0][0].setText("
Input:
000
110
010Output should be
000
000
000by selecting cells \$(0,0)\$, \$(1,0)\$, \$(1,1)\$, \$(2,1)\$, and \$(2,2)\$
I'd like to know if my code can be made any more efficient.
```
import javax.swing.*;
import java.awt.*;
import javax.swing.JFrame;
import java.util.Scanner;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
public class lightOff extends JFrame implements ActionListener
{
public static final int W = 400;
public static final int H = 200;
JButton[][] lights = new JButton[3][3];
int COLS = 3, ROWS = 3;
public lightOff()
{
super("Light Off");
setSize(W,H);
setLayout(new GridLayout(3,3));
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
Scanner kb = new Scanner(System.in);
String[][] input = {{"0","0","0"},{"1","1","0"},{"0","1","0"}};
for(int i=0;i<3;i++)
{
for(int j=0;j<3;j++)
{
lights[i][j] = new JButton(); // Initializing all JButton
lights[i][j].addActionListener(this); // Registering listener
lights[i][j].setText(input[i][j]); // setting text of each
// button as per input String
add(lights[i][j]); // adding JButton to the JFrame
}
}
}
public static void main(String[] args)
{
lightOff obj1 = new lightOff();
obj1.setVisible(true);
}
public void actionPerformed(ActionEvent e)
{
JButton action = (JButton)e.getSource();
if(action==lights[0][0])
{
if("0".equals(lights[0][0].getText()))
lights[0][0].setText("1");
else
lights[0][0].setText("
Solution
I wonder how this generalizes to e.g., a 30x30 game. Would it be 100 times longer?
Why public?
This should be private. Actually, everything should be always private, unless there's good reason against.
These are constants and should be be
Don't call the button "action". I'd suggest
Note the spacing and braces; that's the normal Java style. A single loop instead of the n**2 cases. Now simply call toggle for the selected coordinates and all 4 neighbors (they may be off, but let's handle it by the callee).
And now start by testing if we're in range:
public static final int W = 400;Why public?
JButton[][] lights = new JButton[3][3];This should be private. Actually, everything should be always private, unless there's good reason against.
int COLS = 3, ROWS = 3;These are constants and should be be
private static.public void actionPerformed(ActionEvent e)
{
JButton action = (JButton)e.getSource();
if(action==lights[0][0])Don't call the button "action". I'd suggest
for(int i = 0; i < 3; i++) {
for(int j = 0; j < 3; j++) {
if (source == button[i][j]) {
handle(i, j);
}
}
}Note the spacing and braces; that's the normal Java style. A single loop instead of the n**2 cases. Now simply call toggle for the selected coordinates and all 4 neighbors (they may be off, but let's handle it by the callee).
private void handle(int i, int j) {
toggle(i, j);
toggle(i+1, j);
toggle(i-1, j);
toggle(i, j+1);
toggle(i, j-1);
}And now start by testing if we're in range:
private void toggle(int i, int j) {
if (0 <= i && i < ROWS && 0 <= j && j < COLS) {
JButton toggling = lights[i][j];
String oldText = toggling.getText();
String newText = "1".equals(oldText) ? "0" : "1";
toggling.setText(newText);
}
}Code Snippets
public static final int W = 400;JButton[][] lights = new JButton[3][3];int COLS = 3, ROWS = 3;public void actionPerformed(ActionEvent e)
{
JButton action = (JButton)e.getSource();
if(action==lights[0][0])for(int i = 0; i < 3; i++) {
for(int j = 0; j < 3; j++) {
if (source == button[i][j]) {
handle(i, j);
}
}
}Context
StackExchange Code Review Q#92864, answer score: 10
Revisions (0)
No revisions yet.