简单Java俄罗斯方块游戏

Question

我没有IT背景，自学了Java，制作了一个简单的俄罗斯方块游戏。我把我在互联网上的代码和其他教程进行了比较。我对这些实现的第一印象是它们非常复杂:形状类、无形状形状、旋转数组等等。

形状在网格中的位置也是形状的中心/旋转点。形状是一个相对于形状中央/旋转点的4个点/坐标的数组。

我的代码是不是太简单了？我想要一些反馈。

这是我的代码：

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Component;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.Graphics;
import java.awt.Point;
import java.awt.Toolkit;
import java.awt.event.ComponentAdapter;
import java.awt.event.ComponentEvent;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Random;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JLayeredPane;
import javax.swing.JPanel;

public class Tetris extends JFrame implements Runnable {
    final private static String I = "i";
    final private static String O = "o";
    final private static String J = "j";
    final private static String L = "l";
    final private static String T = "t";
    final private static String S = "s";
    final private static String Z = "z";
    
    final private static HashMap SHAPE = new HashMap();
    final private static HashMap COLOR = new HashMap();
    
    final private int X = 10;
    final private int Y = 20;
    final private int SIZE = Toolkit.getDefaultToolkit().getScreenSize().height / 2 / Y;
    final private int MIN = 100;
    final private int MAX = 750;
    final private int DELAY = 5;
    
    final private JLabel PIECE = new JLabel() {
        @Override
        public void paint(Graphics g) {
            g.setColor((Color) COLOR.get(getText()));
            for (Point p : block) {
                g.fillRect((location.x + p.x) * SIZE, (location.y + p.y) * SIZE, SIZE, SIZE);
            }
        }
    };
    final private JLabel NEXT = new JLabel((String) SHAPE.keySet().toArray()[new Random().nextInt(SHAPE.size())]) {
        @Override
        public void paint(Graphics g) {
            g.setColor((Color) COLOR.get(getText()));
            for (Point p : (Point[]) SHAPE.get(getText())) {
                g.fillRect(getWidth() / 2 + (int) ((p.x - 0.5) * SIZE), (p.y + 2) * SIZE, SIZE, SIZE);
            }
        }
    };
    final private JLabel GAME_OVER = new JLabel("Game over", JLabel.CENTER);
    final private JLabel SCORE = new JLabel("0", JLabel.RIGHT);
    
    final private JButton START = new JButton("Start");
    
    private ArrayList<Color[]> grid = new ArrayList(Arrays.asList(new Color[Y][X]));
    
    private Point location;
    private Point[] block;

    public Tetris() {
        JLayeredPane board = new JLayeredPane();
        JPanel statusPanel = new JPanel(new BorderLayout());
            
        board.setOpaque(true);
        board.setBackground(Color.lightGray);
        board.add(new JPanel() {
            @Override
            public void paint(Graphics g) {
                for (int y = 0; y < Y; y++) {
                    for (int x = 0; x < X; x++) {
                        if (grid.get(y)[x] != null) {
                            g.setColor(grid.get(y)[x]);
                            g.fillRect(x * SIZE, y * SIZE, SIZE, SIZE);
                        }
                    }
                }
            }
        });
        board.add(PIECE, new Integer(1));
        board.add(GAME_OVER, new Integer(2));
        board.setPreferredSize(new Dimension(X * SIZE, Y * SIZE));
        for (Component c : board.getComponents()) {
            c.setSize(board.getPreferredSize());
        }

        statusPanel.add(SCORE, BorderLayout.NORTH);
        statusPanel.add(START, BorderLayout.CENTER);
        statusPanel.add(NEXT, BorderLayout.SOUTH);
        
        PIECE.setVisible(false);
        PIECE.addKeyListener(new KeyAdapter() {
            @Override
            public void keyPressed(KeyEvent e) {
                if (e.getKeyCode() == KeyEvent.VK_LEFT) {
                    blockXY(location.x - 1, location.y, block);
                } else if (e.getKeyCode() == KeyEvent.VK_RIGHT) {
                    blockXY(location.x + 1, location.y, block);
                } else if (e.getKeyCode() == KeyEvent.VK_DOWN) {
                    blockXY(location.x, location.y + 1, block);
                } else if (e.getKeyCode() == KeyEvent.VK_SPACE && !PIECE.getText().equals(O)) {
                    Point[] rotated = new Point[block.length];
                    int x = location.x;
                    for (int i = 0; i < rotated.length; i++) {
                        rotated[i] = new Point(block[i].y, -block[i].x);
                        x = Math.max(-rotated[i].x, Math.min(x, X - 1 - rotated[i].x));
                    }
                    blockXY(x, location.y, rotated);
                }
            }
        });
        PIECE.addComponentListener(new ComponentAdapter() {
            @Override
            public void componentShown(ComponentEvent e) {
                PIECE.requestFocus();
            }
        });
        
        GAME_OVER.setVisible(false);
        GAME_OVER.setFont(new Font(Font.DIALOG_INPUT, Font.BOLD, GAME_OVER.getHeight() / 12));
        
        SCORE.setFont(GAME_OVER.getFont());

        START.setFont(GAME_OVER.getFont());
        START.setBorder(null);
        START.setContentAreaFilled(false);
        START.setFocusPainted(false);
        START.addActionListener(e -> new Thread(this).start());

        NEXT.setPreferredSize(new Dimension(PIECE.getHeight() / 3, PIECE.getHeight() / 3));

        add(board, BorderLayout.WEST);
        add(statusPanel, BorderLayout.EAST);
        
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setResizable(false);
        setVisible(true);
        pack();
    }
    
    private synchronized boolean blockXY(int x, int y, Point[] block) {
        for (Point p : block) {
            if (x + p.x == -1 || x + p.x == X || y + p.y >= Y || (y + p.y > -1 && grid.get(y + p.y)[x + p.x] != null)) {
                return false;
            }
        }
        location = new Point(x, y);
        this.block = block;
        repaint();
        return true;
    }

    @Override
    public void run() {
        START.setVisible(false);
        grid = new ArrayList(Arrays.asList(new Color[Y][X]));        
        SCORE.setText("0");
        GAME_OVER.setVisible(false);
        
        String choice = I + O + J + L + T + S + Z;
        
        do {
            PIECE.setText(NEXT.getText());
            NEXT.setText(choice.split("")[new Random().nextInt(choice.length())]);
            location = new Point(X / 2, -2);
            block = (Point[]) SHAPE.get(PIECE.getText());                
            PIECE.setVisible(true);            

            do {
                try {
                    Thread.sleep(Math.max(MIN, MAX - Integer.parseInt(SCORE.getText()) * DELAY));
                } catch (Exception ex) { }
                PIECE.setVisible(blockXY(location.x, location.y + 1, block));
            } while (PIECE.isVisible());

            for (Point p : block) {
                try {
                    grid.get(location.y + p.y)[location.x + p.x] = (Color) COLOR.get(PIECE.getText());
                } catch (Exception ex) { 
                    GAME_OVER.setVisible(true);
                }
            }

            if (!GAME_OVER.isVisible()) {
                for (int y = 0; y < Y; y++) {
                    if (!Arrays.asList(grid.get(y)).contains(null)) {
                        grid.set(y, new Color[X]);
                        repaint();
                        try {
                            Thread.sleep(800);
                        } catch (Exception ex) { }
                        grid.add(0, grid.remove(y));
                        SCORE.setText("" + (Integer.parseInt(SCORE.getText()) + 1));
                    }
                }
                choice = (I + O + J + L + T + S + Z).replace(NEXT.getText(), NEXT.getText().replace(PIECE.getText(), ""));
            } else {
                try {
                    Thread.sleep(1200);
                } catch (Exception ex) { }
                START.setVisible(true);
                START.requestFocus();
            }
        } while (!GAME_OVER.isVisible());
    }
    
    public static void main(String[] args) {
        SHAPE.put(I, new Point[] {new Point(0, -2), new Point(0, -1), new Point(0, 0), new Point(0, 1)});
        SHAPE.put(O, new Point[] {new Point(0, 0), new Point(-1, 0), new Point(0, 1), new Point(-1, 1)});
        SHAPE.put(J, new Point[] {new Point(0, -1), new Point(0, 0), new Point(0, 1), new Point(-1, 1)});
        SHAPE.put(L, new Point[] {new Point(0, -1), new Point(0, 0), new Point(0, 1), new Point(1, 1)});
        SHAPE.put(T, new Point[] {new Point(-1, 0), new Point(0, 0), new Point(1, 0), new Point(0, 1)});
        SHAPE.put(S, new Point[] {new Point(1, 0), new Point(0, 0), new Point(0, 1), new Point(-1, 1)});
        SHAPE.put(Z, new Point[] {new Point(-1, 0), new Point(0, 0), new Point(0, 1), new Point(1, 1)});
        
        COLOR.put(I, Color.cyan);
        COLOR.put(O, Color.yellow);
        COLOR.put(J, Color.blue);
        COLOR.put(L, Color.orange);
        COLOR.put(T, Color.magenta);
        COLOR.put(S, Color.green);
        COLOR.put(Z, Color.red);
        
        new Tetris().setLocationRelativeTo(null);
    }
    
}

Answer 1

我的代码是不是太简单了？

我们需要对它进行一点解压缩，因为我们可能对“简单”的不同定义进行了操作。您的应用程序只有一个文件和一个顶级类。虽然这在某种意义上很简单，但这并不是一个很好的主意，因为它阻碍了可维护性和可读性，而且实际上在认知负载方面意味着更多的复杂性--理解代码是如何布局的，并且能够对代码进行有意义的修改和测试。您应该尝试细分和模块化。

一个厌倦战斗的程序员对于简单性的看法是完全不同的，它涉及到更小、更好定义的类；更好的关注点分离；等等。

在更细的层面上：

您的单字母常量是一种代码气味，表明您实际上是试图捕捉某种类型的枚举。

我认为SHAPE和COLOR不应该大写；需要<>类型参数；重要的是将它们内联地初始化，而不是在实例之外初始化，比如

    private final Map<String, Point[]> shapes = Map.ofEntries(
        entry(I, new Point[] {new Point( 0, -2), new Point( 0, -1), new Point(0, 0), new Point( 0, 1)}),
        entry(O, new Point[] {new Point( 0,  0), new Point(-1,  0), new Point(0, 1), new Point(-1, 1)}),
        entry(J, new Point[] {new Point( 0, -1), new Point( 0,  0), new Point(0, 1), new Point(-1, 1)}),
        entry(L, new Point[] {new Point( 0, -1), new Point( 0,  0), new Point(0, 1), new Point( 1, 1)}),
        entry(T, new Point[] {new Point(-1,  0), new Point( 0,  0), new Point(1, 0), new Point( 0, 1)}),
        entry(S, new Point[] {new Point( 1,  0), new Point( 0,  0), new Point(0, 1), new Point(-1, 1)}),
        entry(Z, new Point[] {new Point(-1,  0), new Point( 0,  0), new Point(0, 1), new Point( 1, 1)})
    );

    private final Map<String, Color> colors = Map.ofEntries(
        entry(I, Color.cyan),
        entry(O, Color.yellow),
        entry(J, Color.blue),
        entry(L, Color.orange),
        entry(T, Color.magenta),
        entry(S, Color.green),
        entry(Z, Color.red)
    );

colors和shapes具有相同的基数，这意味着需要捕获一个类，其中包含points和color的成员。

表达式grid.get(y)[x]需要进入一个变量以便重用。

不要new Integer(1)，只需写1。

这个谓词：

if (x + p.x == -1 || x + p.x == X || y + p.y >= Y || (y + p.y > -1 && grid.get(y + p.y)[x + p.x] != null)) {

应以行分隔：

        if (x + p.x == -1
            || x + p.x == X
            || y + p.y >= Y
            || (y + p.y > -1 && grid.get(y + p.y)[x + p.x] != null)
        ) {

对您来说，在循环中在new Random()的内部执行run()可能不是一个好主意。您应该有一个Random实例，可能是类中的一个成员，但至少要从该循环中删除。

catch (Exception ex)是程序调试能力的毒药.它需要消失，如果您担心来自try块的一个特定异常，那么就用它代替。

Answer 2

分离关注点

您似乎把所有的代码都放在一个类中，这使得开始时很容易，但是添加的功能越多，就越难。在一个地方拥有所有东西都会降低代码的可修改性和可理解性，因为同时发生的事情太多了。

这就是为什么在分离不同关注点的过程中存在很多模式，这样它们就可以相互独立地修改。我喜欢MVVM。在MVVM中，至少有3个类:一个用于逻辑，一个用于UI和一个连接层。

长方法

您的方法非常长，有多个层次的嵌套。这可能是很难阅读和理解。

我喜欢使用以下一般的经验法则：

• 该方法是否大于5行？-> 提取是其中的一部分。
• 该方法是否有多个层次的缩进？例如嵌套的ifs或循环？->提取，直到它只有一个级别缩进。

将代码块提取到函数中使您可以为该块指定名称。这个名称应该清楚地说明块的意图，而不是实现。这样，只有当读者还想知道方法是如何实现的时候，读者才不需要阅读方法内容才能理解该方法正在做什么。

要获得更多的深度信息，您可以查看我在codereview元版上发表的关于可读代码的文章。