HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

"Lights off" puzzle

Submitted by: @import:stackexchange-codereview··
0
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:

000
110
010


Output should be

000
000
000


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("

Solution

I wonder how this generalizes to e.g., a 30x30 game. Would it be 100 times longer?

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.