1
\$\begingroup\$

This is improved code from my previous question. This mini game which we call "Moon Buggy" is available in beta from the google playstore.

enter image description here

The action is that you control a vechicle on the moon and you defend yourself against evil UFO:s.

I have written a separate class for the UFO which is instanciated once for every UFO:

import android.content.Context;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.graphics.Canvas;
import android.graphics.Paint;
import android.os.Handler;
import android.os.Looper;

import java.util.Random;

public class AttackingAlien extends Alien {

    private Bitmap alien;
    static boolean recent = true; // switch this in beginning
    private long changeDirections = System.currentTimeMillis();
    private long fireTimeout = System.currentTimeMillis();
    private int screenWidth;
    private int screenHeight;
    private int ufoY = 0;
    private int ufoX = 0;
    private int missileX = 25;
    private int deltaUfoY = 7;
    private int deltaUfoX = 7;
    private int missileOffSetY = 0;
    private int missileYstart = 0;
    private boolean wasHit = false;
    private boolean alienexplode;
    private boolean waitForTimer, waitForUfoTimer;
    private boolean toggleDeltaY = true;
    private boolean runOnce = true;
    private boolean startMissile = true;

    public AttackingAlien(ParallaxView view, Context context, String name, final int screenWidth, int screenHeight, int p) {
        super(context, name);
        this.deltaUfoY = p;
        int alienResID = context.getResources().getIdentifier(name,
                "drawable", context.getPackageName());
        alien = BitmapFactory.decodeResource(context.getResources(), alienResID);
        int max = (int) (0.75 * screenWidth);
        int min = 20;
        int diff = max - min;
        Random rn = new Random();
        int i5 = rn.nextInt(diff + 1);
        i5 += min;
        missileX = i5;
        ufoX = missileX;
        max = 200;
        diff = max - min;
        this.screenHeight = screenHeight;
        this.screenWidth = screenWidth;
        ufoY = 0;
        waitForUfoTimer = true;

        int max2 = 20000;
        int min2 = 18000;
        int diff2 = max2 - min2;
        Random rn2 = new Random();
        int result = rn2.nextInt(diff2 + 1);
        result += min2;

        Handler handler = new Handler(Looper.getMainLooper());
        handler.postDelayed(new Runnable() {
            @Override
            public void run() {
                missileX = ufoX;
                setRecent();
                waitForUfoTimer = false;
            }
        }, result);
    }

    private void changeDirections() {

        int max2 = 500;
        int min2 = 100;
        int diff2 = max2 - min2;
        Random rn2 = new Random();
        int result = rn2.nextInt(diff2 + 1);
        result += min2;

        if (System.currentTimeMillis() - changeDirections >= result) {
            // Change direction here
            toggleDeltaY = !toggleDeltaY;
            changeDirections = System.currentTimeMillis();
        }
    }

    public void update(Canvas canvas, Paint paint, boolean toggleDeltaY, int screenWidth, int screenHeight) {
        if (ufoX > screenWidth - 250 || ufoX < 10) { // UFO change horizontal direction
            deltaUfoX = -deltaUfoX;
        }

        //make sure UFO does not move too low
        if (ufoY >= 20) {
            deltaUfoY = -deltaUfoY;
        }
        if ((ufoY + screenHeight / 100 * 25) <= 0) // don't move outside the top
            deltaUfoY = -deltaUfoY;

        if (!waitForUfoTimer && Background.checkpoint >= 'A') { // && sectionComplete > 0) {
            runOnce = true;
            //alienY++;
            canvas.drawBitmap(alien, ufoX + 10, ufoY + screenHeight / 100 * 25, paint);
        }
        //missileX = missileX + speedAlienX;
        ufoX = ufoX + deltaUfoX;
        if (waitForTimer) missileX = ufoX;
        if (toggleDeltaY) {
            deltaUfoY = -deltaUfoY;
        }
        ufoY = ufoY + deltaUfoY;
        changeDirections();
    }

    public void checkBeingHit(int[] missiles, int buggyXDisplacement, double buggyXDistance, Canvas canvas, Bitmap explode2, Paint paint, int score, ParallaxView pview, int i1, int xbuggy2) {
        // if UFO is being hit by buggy
        if (!waitForTimer && java.lang.Math.abs(ufoX + 10 - 400 - buggyXDistance) * 2 < (alien.getWidth()) && java.lang.Math.abs(ufoY + screenHeight / 100 * 25 - (screenHeight / 100 * 95 - missiles[i1] - xbuggy2)) * 2 < (alien.getHeight())) {
            missileOffSetY = -9999;
            canvas.drawBitmap(explode2, ufoX + 10, ufoY + screenHeight / 100 * 25, paint);
            if (runOnce) {
                ParallaxView.score = ParallaxView.score + 100;
                Handler handler = new Handler(Looper.getMainLooper());
                handler.postDelayed(new Runnable() {
                    @Override
                    public void run() {
                        int max = (int) (0.75 * screenWidth);
                        int min = 20;
                        int diff = max - min;
                        Random rn = new Random();
                        int i5 = rn.nextInt(diff + 1);
                        i5 += min;
                        missileX = i5;//25;
                        ufoX = missileX;
                        ufoY = 0;
                        alienexplode = false;
                        waitForTimer = false;
                        waitForUfoTimer = false;
                        startMissile = true;
                    }
                }, 3000);
            }
            runOnce = false;
            waitForUfoTimer = true;
            startMissile = false;
            waitForTimer = true;
            if (!alienexplode) {
                pview.changeText();
            }
            alienexplode = true;
        }
    }

    private void checkFire() {
        int max = 15000;
        int min = 12000;
        int diff = max - min;
        Random rn = new Random();
        int i5 = rn.nextInt(diff + 1);
        i5 += min;
        if (System.currentTimeMillis() - fireTimeout >= i5) { // means how often the alien fires
            fireTimeout = System.currentTimeMillis();
            missileOffSetY = 0;
            missileX = ufoX;
        }
    }

    private void setRecent() {
        AttackingAlien.recent = false;
    }



    public boolean drawMissile(ParallaxView view, Canvas canvas, Paint paint, int buggyXDisplacement, double buggyXDistance, Bitmap buggy, int jumpHeight, int screenHeight) {
        wasHit = false;
        checkFire();
        // if buggy was hit by a missile
        if (!AttackingAlien.recent && !view.waitForTimer && java.lang.Math.abs(((buggyXDisplacement + buggyXDistance) + buggy.getWidth() / 2) - (missileX + 10 + alien.getWidth() / 2)) < buggy.getWidth() / 2 && java.lang.Math.abs((ufoY + screenHeight / 100 * 25 + 75 + missileOffSetY) - ((screenHeight * 0.3) - jumpHeight + buggy.getHeight())) < 65) {
            AttackingAlien.recent = true;
            canvas.drawBitmap(view.explode, (float) (buggyXDisplacement + buggyXDistance), (float) (screenHeight * 0.5) - jumpHeight, paint);
            ParallaxView.bombed--;
            missileOffSetY = 0;
            wasHit = true;
            view.recent = true;
            Handler handler = new Handler(Looper.getMainLooper());
            handler.postDelayed(new Runnable() {
                @Override
                public void run() {
                    setRecent();
                    waitForTimer = false;
                    wasHit = false;
                }
            }, 7000);
            waitForTimer = true;
        } else {
            // buggy was not hit so UFO fires more missiles
            //TODO: check if the movements are realistic
            if (!waitForTimer && !waitForUfoTimer && Background.checkpoint >= 'A') {
                if (startMissile) {
                    startMissile = false;
                    missileYstart = ufoY;
                }
                canvas.drawText("●", missileX + alien.getWidth() / 2, missileYstart + screenHeight / 100 * 25 + alien.getHeight() + missileOffSetY, paint);
                missileOffSetY = missileOffSetY + 4;
            }
            wasHit = false;
        }
        return wasHit;
    }
}

The remainder of the code is available on request.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$
  1. You should remove duplicated blocks.

    int max2 = 20000;
    int min2 = 18000;
    int diff2 = max2 - min2;
    int result = rn2.nextInt(diff2 + 1);
    result += min2;
    

    is repeated many times with slight variation in values of min and max. you can extract it as a function and pass the min-max values as parameters.

  2. You can replace your anonymous Runnable classes with lambda expressions if you are using Java 8 or above. Or extract them as inner class with a meaningful name.
  3. value of diff at line 48 isn't used anywhere in the remaining code.
  4. The if statement of method drawMissile at line 177 is too big to understand. Consider extracting it to a method with meaning full name, that explains the condition.
  5. The constructor is too long and has too many parameters. And the variable p isn't a very useful variable name. You may group variables together in one class if they always come together, like screeHeight and screenWidth. Like we create a separate Address class to store address fields and methods, instead of keeping address fields and methods in User class or Employee class. Always group similar variables and methods together in a separate class.
  6. The update method updates what? It isn't clear with the name if it will update Alien's position, UFO's position, Missile's positions, or anything else.
  7. Method checkBeingHit too is very long and has a lot of parameters. Consider breaking down the method into smaller methods and reduce the parameter count by grouping similar variables together.
    1. Same with drawMissile.
  8. It can be renamed to isHit that returns a boolean to show if the alien was hit or not, instead of having return type as void.
  9. Same with checkFire.
  10. setRecent doesn't take any parameter to set the value of recent. Is it possible that you meant resetRecent?
\$\endgroup\$
2
  • \$\begingroup\$ I cannot guess lambda syntax for Java 8 in this case. \$\endgroup\$ Commented Jul 8, 2018 at 8:39
  • \$\begingroup\$ @NiklasRosencrantz, it was just a suggestion if you were able to trim down the method of the anonymous class to one or two line. Else you can extract the whole anonymous block to inner class or a new class. \$\endgroup\$
    – Ankit Soni
    Commented Jul 8, 2018 at 23:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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