5
\$\begingroup\$

I made an Agario single player version (soon to become a multi player one using socket). What can be some improvements?

Controller:

package agario;

import java.awt.Color;
import java.awt.Frame;
import java.awt.Graphics;
import java.awt.Rectangle;
import java.awt.event.MouseEvent;
import java.awt.event.MouseMotionAdapter;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Random;

public class Controller {
    ArrayList<Blob> blobs = new ArrayList<Blob>();
    public ArrayList<Dot> dots = new ArrayList<Dot>();
    Blob b = new Blob(50,50,30,Color.CYAN);
    MyFrame mf = new MyFrame("Agario");
    int mouseX = 0;
    int mouseY = 0;
    static int score = 30;
    static int xDis = 0;
    static int yDis = 0;
    public static void main(String[] args){
        new Controller().startGame();
    }
    public void startGame(){
        mf.addMouseMotionListener(new MyMouseMoveListener());

        Refresh rf = new Refresh();
        Thread t = new Thread(rf);
        t.start();

        while(true){
            try{
                Random r = new Random();
                Thread.sleep(r.nextInt(40));

                double dis = Math.sqrt(xDis*xDis + yDis*yDis);
                double easingAmount = 180/b.size;
                if(dis > 1){
                    b.x += easingAmount*xDis/dis;
                    b.y += easingAmount*yDis/dis;
                }

                if(r.nextInt(10) == 5){
                    int randX = r.nextInt(600);
                    int randY = r.nextInt(600);
                    Dot d = new Dot(randX,randY);
                    synchronized(dots){
                        dots.add(d);
                    }
                    mf.add(d);
                    mf.repaint();
                    System.out.println(score);
                }
            }catch(Exception e){

            }
        }
    }
    class Refresh implements Runnable{

        public void run() {
            while(true){
                Random ran = new Random();
                try{
                    Thread.sleep(ran.nextInt(20));
                }catch(Exception e){
                    System.out.println("error");
                }
                Rectangle r = new Rectangle(b.x,b.y,b.size,b.size);
                synchronized(dots){
                    Iterator i = dots.iterator();
                    while(i.hasNext()){
                        Dot d = (Dot) i.next();
                        Rectangle r1 = new Rectangle(d.x,d.y,12,12);
                        if(r1.intersects(r)){
                            i.remove();
                            b.size += 1;
                            score += 1;
                        }
                    }
                }
                mf.repaint();
            }

        }
    }
    class MyMouseMoveListener extends MouseMotionAdapter{
        public void mouseMoved(MouseEvent m){
            mouseX = m.getX();
            mouseY = m.getY();
            xDis = mouseX-b.x;
            yDis = mouseY-b.y;
        }
    }
    class MyFrame extends Frame{
        MyFrame(String s){
            super(s);
            setBounds(0,0,900,900);
            add(b);
            blobs.add(b);
            setVisible(true);
        }
        public void paint(Graphics g){
            for(Blob b : blobs)
                b.paint(g);
            synchronized(dots){
                Iterator i = dots.iterator();
                while(i.hasNext()){
                    Dot d = (Dot) i.next();
                    d.paint(g);
                }
            }
        }
    }
}

Dot Object:

package agario;

import java.awt.Color;
import java.awt.Component;
import java.awt.Graphics;
import java.util.Random;

public class Dot extends Component{
    public int x;
    public int y;
    public Color c;

    Dot(int x, int y){
        this.x = x;
        this.y = y;
        Random rand = new Random();
        int r = rand.nextInt(255);
        int green = rand.nextInt(255);
        int b = rand.nextInt(255);
        this.c = new Color(r,green,b);
    }
    public void paint(Graphics g){
        g.setColor(c);
        g.fillOval(this.x, this.y, 10, 10);
    }
}

Blob object:

package agario;

import java.awt.Color;
import java.awt.Component;
import java.awt.Graphics;
import java.util.Random;

public class Blob extends Component{
    public int x;
    public int y;
    public int size;
    public Color color;
    public static Blob blob = new Blob(800,100,40,Color.blue);
    Blob(int x, int y, int size, Color c){
        this.x = x;
        this.y = y;
        this.size = size;
        this.color = c;
    }
    public void paint(Graphics g){

        g.setColor(color);
        g.fillOval(x, y, size, size);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Changing it to multiplayer using socket will also be greatly appreciated! Thanks. \$\endgroup\$ – Meinkraft Oct 4 '15 at 4:19
  • 8
    \$\begingroup\$ We can make sure your code is flexible enough to allow extending it to use sockets for multiplayer, but the implementation of that is on you: a peer review doesn't add features. \$\endgroup\$ – Mathieu Guindon Oct 4 '15 at 4:32
9
\$\begingroup\$

Make members final

final member variables are easy to work with. As they can never be reassigned, they are less error prone, as accidental reuses are not possible.

For example, in Dot, you can make all members final. In Controller, you can make blobs, dots, b, mf members final.

Make members private

It's a good policy to make member variables private by default, and relax the visibility only when needed. It keeps your interfaces cleaner, by hiding internal details that users of the class don't need to see. It also helps avoiding bugs.

For example in Controller, all members can be private.

Eliminate pointless members

In Controller, the members mouseX and mouseY can be converted to local variables in the MyMouseMoveListener inner class.

In Blob, the member blob is never used, so it can be safely deleted.

Don't catch Exception

It's a very bad practice to catch Exception. Whenever you can, always catch the most specific kind of Exception that you expect in the code block. Otherwise it might catch exceptions that you didn't really expect, and which indicate a bug in the program and you might never know.

Don't ignore exceptions

It's a very bad practice to use an empty catch block when handling exceptions. At the minimum, leave a comment why it is "OK" to do nothing.

Generating random numbers

There's no need to create multiple Random objects in a loop. Create a single Random object before the loop, and reuse it. So instead of:

while (true) {
    Random random = new Random();
    int num = random.nextInt(40);

This is better:

Random random = new Random();
while (true) {
    int num = random.nextInt(40);

Avoid raw types

There's really no excuse for using a raw Iterator and an ugly cast here:

Iterator i = dots.iterator();
while (i.hasNext()) {
    Dot d = (Dot) i.next();
    d.paint(g);
}

Using parameterized types properly:

Iterator<Dot> i = dots.iterator();
while (i.hasNext()) {
    Dot d = i.next();
    d.paint(g);
}           

Use enhanced for-each to iterate over a collection

This is an archaic, tedious way to iterate over a collection:

Iterator i = dots.iterator();
while (i.hasNext()) {
    Dot d = (Dot) i.next();
    d.paint(g);
}

This is an idiomatic, better way:

for (Dot d : dots) {
    d.paint(g);
}
\$\endgroup\$
  • \$\begingroup\$ Non-final fields are completely predictable - they change when you change them. I'd maybe say instead that final makes it harder to accidentally confuse yourself, by changing fields when you didn't mean to. \$\endgroup\$ – user253751 Oct 4 '15 at 10:42
  • \$\begingroup\$ Alright, I reworded it. \$\endgroup\$ – Stop ongoing harm to Monica Oct 4 '15 at 11:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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