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

Swing UI to display and edit terrain for a game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
editswinggameforanddisplayterrain

Problem

I would like someone to review my code and maybe give me some advice to maintain it because I'm still a beginner.

```
public static boolean NewTerrainCamPos = false;

public static String textVal;
public static String textVal2;
public static String resiveTex = "1";
public static String resiveTex2;

public static final int Width = 1000;
public static final int Height = 720;
public static final int FPS_CAP = 120;

private static long lastFrameTime;
private static float delta;

public static void createDisplay(){

ContextAttribs attribs = new ContextAttribs(3,2).withForwardCompatible(true).withProfileCore(true);

try {
Canvas openglSurface = new Canvas();
JFrame frame = new JFrame();
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.setLayout(new BorderLayout());

//.............................
JMenuBar menuBar = new JMenuBar();
JMenu terrain = new JMenu("Terrain");
menuBar.add(terrain);
menuBar.add(terrain);
JMenuItem exit = new JMenuItem("Exit");
JMenuItem newTerrain = new JMenuItem("add Terrain");
JMenuItem editTerrain = new JMenuItem("Edit Terrain");

newTerrain.addActionListener(new ActionListener(){
public void actionPerformed(ActionEvent e) {
NewTerrainCamPos = true;
JFrame frame2 = new JFrame();
frame2.setVisible(true);
frame2.setSize(300, 300);
//...............................
GridLayout experimentLayout = new GridLayout(3,2);
frame2.setLayout(experimentLayout);
//.....................................
JLabel xCord = new JLabel("XCoords: ");
JLabel zCord = new JLabel("ZCoords: ");
JTextField text = new JTextField();
JTextField text2 = new

Solution

Nitpicks:

-
Your indentation isn't quite correct. It seems to have too many levels in some places, and it's incorrect here:

try {
     Canvas openglSurface = new Canvas();
        JFrame frame = new JFrame();


-
static final fields should always be SHOUT_CASE. WIDTH and HEIGHT instead of Width and Height

-
Leave some space between operators when you do calculations: return Sys.getTime()1000/Sys.getTimerResolution(); is rather unreadable compared to return Sys.getTime() 1000 / Sys.getTimerResolution();

-
Your capitalizaion between "Edit Terrain" and "add Terrain" is different. I suggest you stick to Title Case

-
Don't use Rawtypes. JList should be qualified with a generic type. Use JList instead.

Access and Scope

All the fields declared public probably can (and should) be private. You shouldn't be using them outside this class. This means you should make them inaccessible -> private.

Aside from that you're overusing the static context. I'm feeling that I sound like a broken record, but... Avoid static like the plague. It goes against good OOP principles to use static fields for anything but actual constants, so don't do it.

This advice applies to all of the code you have here except FPS_CAP, WIDTH and HEIGHT.

Naming

-
Don't shorthand names. ContextAttribs should be ContextAttributes.

-
Don't number your variables. That's semantically unhelpful. What's the difference between frame, frame2 and frame3?? How will you understand the difference if you don't see the full code?

Same goes for text and text2.

-
Don't name things after their type. JList list is incredibly unuseful. It provides no information whatsoever

Code Snippets

try {
     Canvas openglSurface = new Canvas();
        JFrame frame = new JFrame();

Context

StackExchange Code Review Q#124314, answer score: 2

Revisions (0)

No revisions yet.