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

Single-player Agario game

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

Problem

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 blobs = new ArrayList();
public ArrayList dots = new ArrayList();
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(xDisxDis + yDisyDis);
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();

Solution

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 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);
}

Code Snippets

while (true) {
    Random random = new Random();
    int num = random.nextInt(40);
Random random = new Random();
while (true) {
    int num = random.nextInt(40);
Iterator i = dots.iterator();
while (i.hasNext()) {
    Dot d = (Dot) i.next();
    d.paint(g);
}
Iterator<Dot> i = dots.iterator();
while (i.hasNext()) {
    Dot d = i.next();
    d.paint(g);
}
Iterator i = dots.iterator();
while (i.hasNext()) {
    Dot d = (Dot) i.next();
    d.paint(g);
}

Context

StackExchange Code Review Q#106499, answer score: 10

Revisions (0)

No revisions yet.