13
\$\begingroup\$

I have a little Mario game I started working on and I realized that its format is very messy. Can anyone critique my code on how to make it better, faster, neater, and easier to access?

frame.java

package EvilMario;                                                                           //Include this class in the EvilMario game package

import javax.swing.JFrame;                                                                   //Import the JFrame

public class frame {                                                                         //Run this class to run the game
        public static void main(String[] args) {                                                 //The first method called by java
                JFrame frame = new JFrame("EvilMario v.1.0.2 by Max Mastalerz");                     //Create JFrame called frame

                frame.getContentPane().add(new board());                       //Go to board class
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);                                //Make frame close on X click
                frame.setSize(600,413);                                                              //Set the frame size to the size of the background
                frame.setResizable(false);                                                           //Make sure the user can't resize the frame
                frame.setLocation(20, 50);                                                           //Place the frame in a nicer position
                frame.setVisible(true);                                                              //Make the frame visible

                int frameWidth  = frame.getContentPane().getWidth();
                int frameHeight  = frame.getContentPane().getHeight();
        }
}

board.java

package EvilMario;                                                                           //Include this class in the EvilMario game package

import java.awt.*;                                                                           //Imported to allow use of Image
import java.awt.event.*;                                                                     //Imported to allow use of ActionListener

import javax.swing.*;

public class board extends JPanel implements ActionListener {
        player p;                                                                                //Instance of player class
        Image background, menuBg;                                                                //The background images
        Timer time;                                                                              //A timer
        private menu Menu;
        private frame Frame;

        public static enum STATE {MENU,GAME};

        public static STATE State = STATE.MENU;

        public board() {
                this.addMouseListener(new mouseInput());
                p = new player();                                                                    //Start running player class
                Menu = new menu();

                addKeyListener(new AL());                                                            //Listen for keys
                setFocusable(true);                                                                                                                  //Allows movement         
                ImageIcon i = new ImageIcon("D:/ICS3U1/EvilMario/images/Menu.jpg");                  //Image for menu
                menuBg = i.getImage();
                i = new ImageIcon("D:/ICS3U1/EvilMario/images/EvilMario_Background.png");  //Image for background
                background = i.getImage();                                                           //Give the background the image
                time = new Timer(20,this);                                                           //Timer set to update "this" class every 20 milliseconds(Approximately 50fps)
                time.start();                                                                        //Actually start the timer
        }

        public void actionPerformed(ActionEvent e) {
                p.move();                                                                            //Call the move method from the player class
                repaint();                                                                           //Repaint
        }

        public void paintComponent(Graphics g) {                                                 //Graphics method
                if(State==STATE.GAME) {
                        super.paintComponent(g);
                                Graphics2D g2d = (Graphics2D) g;                                             //casts 2d graphics(or however you would explain it)

                                g2d.drawImage(background, -p.nx, 0, null);                                   //Draw the background image
                                g2d.drawImage(background, -p.nx2, 0, null);                                  //Draw the background image

                                if(-p.nx<-575)                                                              //If going forwards
                                        p.nx=-575;                                                              //Start placing forwards every 575px in front on the last one
                                else if(-p.nx>575)                                                          //If going backwards
                                        p.nx=575;                                                               //Start placing backwards every 575px behind the last one

                                if(-p.nx2<-575)                                                             //If going forwards
                                        p.nx2=-575;                                                             //Start placing forwards every 575px in front on the last one
                                else if(-p.nx2>575)                                                         //If going backwards
                                        p.nx2=575;                                                              //Start placing backgrounds every 575px behind the last one

                                g2d.drawImage(p.getImage(), p.getX(), p.getY(), null);                      //Draw the player at the position he is currently(Coordinate values taken from player class)
                } else {
                        g.drawImage(menuBg, 0, 0, null);
                        menu.render(g);
                }
        }

        private class AL extends KeyAdapter {                                                    //Action Listener extends key adapter
                public void keyPressed(KeyEvent e) {                                                 //On key press
                        p.keyPressed(e);                                                                 //Send whatever key was pressed  TO the keyPressed  method in the player class
                }
                public void keyReleased(KeyEvent e) {                                                //On key release
                        p.keyReleased(e);                                                                //Send whatever key was released TO the keyReleased method in the player class
                }
        }
}

player.java

package EvilMario;                                                                               //Include this class in the EvilMario game package

import java.awt.Image;
import java.awt.event.KeyEvent;

import javax.swing.ImageIcon;

public class player {
    int x, dx, y, nx, nx2, distanceTraveled;                                                     //x coordinate,change in x coordinate,y coordinate,1st rep bg,2nd rep bg,dist traveled
    Image player;                                                                                //The player variable
    ImageIcon playerFacingLeft = new     ImageIcon("D:/ICS3U1/EvilMario/images/MarioLeft.png");  //Image for player while he is     turning left
    ImageIcon playerFacingRight = new     ImageIcon("D:/ICS3U1/EvilMario/images/MarioRight.png");//Image for player while he is turning right

        public player() {
            player = playerFacingRight.getImage();                                               //Give the player the image
            x = 75;                                                                              //The original x position of the player
            y = 285;                                                                             //The original y position of the player
            nx = -0;                                                                             //Repeating background 1
            nx2 = -575;                                                                          //Repeating background 2
            distanceTraveled = 24;
        }

        public void move() {
            if(x>0 && x<300) {                                                                   //If the player is within the moving area
                x = x+dx;                                                                        //The x position is updated to become itself+the amount you moved
                nx = nx+dx;                                                                      //Place the repeating background at regular speed
                nx2 = nx2+dx;                                                                    //Place the repeating background at regular speed
            }
            if(x<=0) {                                                                           //If the player has reached he very left side of the screen(0px)
                x=1;                                                                             //Move him up a pixel so he can move again
                nx = nx+(dx*(int)0.5);                                                           //Place the background at a slower speed since Mario stops moving
                nx2 = nx2+(dx*(int)0.5);                                                         //Place the background at a slower speed since Mario stops moving
            }
            if(x>=300) {                                                                         //If the player has reached the center of the screen(300px)
                x=299;                                                                           //Move him down a pixel so he can move again
                nx = nx+(dx*(int)0.5);                                                           //Place the background at a slower speed since Mario stops moving
                nx2 = nx2+(dx*(int)0.5);                                                         //Place the background at a slower speed since Mario stops moving
            }
            if(dx>0)distanceTraveled++;else if(dx<0)distanceTraveled--;
            if(distanceTraveled>104)x=299;
            if(x==1 && dx<0)
                distanceTraveled++;
            if(distanceTraveled<104){
                nx=0;
                nx2=-575;
            }
        }

        public int   getX()     { return x;      }                                               //This method will return the x.      Is used by other classes
        public int   getY()     { return y;      }                                               //This method will return the y.      Is used by other classes
        public Image getImage() { return player; }                                               //This method will return the player. Is used by other classes

        public void keyPressed(KeyEvent e) {                                                     //Called from the board class, the argument is whatever key was pressed
            int key = e.getKeyCode();                                                            //The key originally sent from the board class

            if(key == KeyEvent.VK_LEFT) {                                                        //If the key sent was LEFT
                player = playerFacingLeft.getImage();                                            //Make the player face leftwards
                if(distanceTraveled<104)dx=-3;else dx=-2;
            }

            if(key == KeyEvent.VK_RIGHT) {                                                       //If the key sent was RIGHT
                player = playerFacingRight.getImage();                                           //Make the player face rightwards
                if(distanceTraveled<104)dx=3;else dx=2;
            }
        }

        public void keyReleased(KeyEvent e) {                                                    //Called from the board class, the argument is whatever key was released
             int key = e.getKeyCode();                                                           //The key originally sent from the board class

            if(key == KeyEvent.VK_LEFT || key == KeyEvent.VK_RIGHT)                              //If the left or right key was released
                dx = 0;                                                                          //Stop moving
        }
}

menu.java

package EvilMario;

import java.awt.Color;
import java.awt.Font;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Rectangle;

public class menu {
    public static Rectangle playButton = new Rectangle(10, 50, 100, 37);
    public static Rectangle helpButton = new Rectangle(10, 102, 100, 37);
    public static Rectangle quitButton = new Rectangle(10, 154, 100, 37);

    public static void render(Graphics g) {
        Graphics2D g2d = (Graphics2D) g;

        Font fnt0 = new Font("arial", Font.BOLD, 45);
        Font fnt1 = new Font("arial", Font.BOLD, 30);
        Font fnt2 = new Font("arial", Font.BOLD, 15);
        g.setFont(fnt0);
        g.setColor(Color.white);
        g.drawString("EvilMario", 10, 40);

        g.setFont(fnt1);
        g.drawString("Play", playButton.x + 19, playButton.y + 29);
            g2d.draw(playButton);
        g.drawString("Help", helpButton.x + 19, helpButton.y + 29);
            g2d.draw(helpButton);
        g.drawString("Quit", quitButton.x + 19, quitButton.y + 29);
            g2d.draw(quitButton);

        g.setFont(fnt2);
        g.drawString("Version 0.0.2",500,380);
    }
}

mouseInput.java

package EvilMario;

import java.awt.Rectangle;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;

public class mouseInput implements MouseListener{
    public void mousePressed(MouseEvent e) {
        int mx = e.getX();
        int my = e.getY();

        if(mx>=10 && mx<=110) {
            if(my>=50 && my<=87) {
                board.State = board.STATE.GAME;
            }
            if(my>=102 && my<=139) {
                //HELP
            }
            if(my>=154 && my<=191) {
                System.exit(1);
            }
        }
    }
    public void mouseClicked(MouseEvent e) {}
    public void mouseReleased(MouseEvent e) {}
    public void mouseEntered(MouseEvent e) {}
    public void mouseExited(MouseEvent e) {}
}

If you wish to test out the game, here are the picture files:

Menu

Background

MarioLeft.png

MarioRight.png

I commented a lot of lines so you can understand the game logic.

\$\endgroup\$
8
\$\begingroup\$

There's two glaring issues jumping into my face in this code. They are both somewhat beginner mistakes, so don't be sad :)

Commenting:

"I commented a lot of lines so you can understand the game logic"
That is not the purpose of comments. This is a common misconception, especially when beginning programming. When writing code, you should strive to comment the lest possible.

There are two exceptions to this rule:

  1. Mandatory comments:
    This especially concerns code you write for companies. There may be some legal requirements that need to be commented. Prominent example: License and author.

  2. Javadoc:
    When writing code for public usage, you should at some point explain what the publicly visible methods do. This is done using Javadoc, a special kind of comments.

Other than that, most comments are pure noise.

//Include this class in the EvilMario game package

I see that myself ;) It's the same for all the comments you placed on the side of your files. It needlessly lengthens lines and makes your code much harder to read effectively :(

Naming:

Part I: Conventions

Java has some naming conventions.

  • "Classes should be named in PascalCasing". Your classes all begin with a lower-case letter. If you create a new class in eclipse, and the first letter is lowercase, the "Create Class" dialog will produce a warning: "Lower case type names are discouraged in Java".

  • "Fields and methods should be named in camelCase". Most of your fields are named in PascalCase. This partly is connected to the fact, that your types are named camelCase already. If you swap that, your code becomes much more Java-like ;)

  • Your Enum-type is named in ALLCAPS. Enums belong to the types. They also are supposed to be named in PascalCase. But your enum values are nicely ALL_CAPS. I like that. (Take with a grain of salt, that is my personal preference)

Part II: Variable names:

int x, dx, y, nx, nx2;
Graphics g;
Font fnt0, fnt1, fnt2;

All these variable names are less than optimal... what does fnt mean? (Font, I know..) Which fnt is which font, where should they be used?

What does g mean? g-force? \$9,81 \dfrac{m}{s^2}\$ (gravitational factor of earth)? what is dx, nx, nx2?

You see what I am getting at? These variable names need to be more descriptive.

fnt0 --> arialLarge
fnt1 --> arialMedium
fnt2 --> arialSmall
g    --> graphicsDevice
dx   --> deltaX /  xChange
nx   --> newX (?)
nx2  --> secondNewX(?)

Final words:

Your code is showing you are a beginner in Java. That's not a bad thing, but your style is somewhat messy. Improve your variable naming and follow the Java-conventions. Comment less. Try to make your code so easy to read, that commenting feels useless, or better even, a hindrance for reading.

\$\endgroup\$
5
  • \$\begingroup\$ The author had the good sense to put all of those comments in the margin, though, so at least they are unobtrusive. \$\endgroup\$ May 19 '14 at 17:46
  • \$\begingroup\$ seriosuly... I hate commenting so much too... but this is for school and yeah.... \$\endgroup\$ May 19 '14 at 19:20
  • 3
    \$\begingroup\$ You should mention that vowels are not expensive, contrary to what a lot of people think while programming. \$\endgroup\$
    – skiwi
    May 19 '14 at 19:21
  • \$\begingroup\$ PascalCase or CamelCase? Or are they same? \$\endgroup\$ May 21 '14 at 7:39
  • 1
    \$\begingroup\$ @ambigram_maker PascalCase is camelCase starting with an uppercase letter. camelCase always starts with a lowercase letter. \$\endgroup\$
    – Vogel612
    May 21 '14 at 8:04

Not the answer you're looking for? Browse other questions tagged or ask your own question.