Java programming style advice

Question

I did a lot of Java programming and then dropped it and did a bunch of ruby. Now I am back in Java and I am wondering whether my programming style is strange.

My code below feels very verbose to me, and the question is it reasonably idiomatic Java? Any suggestions/improvements that you would recommend?

    final static int FORCE_RIGHT = 0;
    final static int FORCE_DOWN = 1;
    final static int FORCE_LEFT = 2;
    final static int FORCE_UP = 3;
    final static int IMP_RIGHT = 4;
    final static int IMP_DOWN = 5;
    final static int IMP_LEFT = 6;
    final static int IMP_UP = 7;


    public void applyForce(int dir) {
        counter++;
        Vector2 vect = new Vector2();
        switch (dir) {
            case FORCE_RIGHT: vect = new Vector2(3.0f, 0.0f); break;
            case IMP_RIGHT: vect = new Vector2(1.0f, 0.0f); break;
            case FORCE_LEFT: vect = new Vector2(-3.0f, 0.0f); break;
            case IMP_LEFT: vect = new Vector2(-1.0f, 0.0f); break;
            case FORCE_UP: vect = new Vector2(0.0f, -3.0f); break;
            case IMP_UP: vect = new Vector2(0.0f, -1.0f); break;
            case FORCE_DOWN: vect = new Vector2(0.0f, 3.0f); break;
            case IMP_DOWN: vect = new Vector2(0.0f, 1.0f); break;
        }
        Vector2 place = body.getWorldCenter();
        if (dir == FORCE_RIGHT || dir == FORCE_LEFT || dir == FORCE_DOWN || dir == FORCE_UP) 
            { 
            body.applyForce(vect, place);
            }
        else 
            { 
            body.applyLinearImpulse(vect, place);
            }
        Log.v("CAR", "Applied force: " + dir + "("+counter+")");
    }

Answer 1

Here are my improvement suggestions:

• enums are smarter, since you can also apply behaviour, for example in your log you'd not only see a number
• instead of the switch statement you could apply a more generic algorithm, but it's not necessarily better, since it then is more difficult to understand and change, even if it comes with less lines of code
• no need to do new Vector2() if you call the constructor again afterwards
• use consistent code formatting (e.g. place to put {, space charachters, ...)

Answer 2

It might be a better idea to convert all your static integers to an enum.

Answer 3

You might consider using an enum, e.g.,

public enum Force {
    FORCE_RIGHT (3.0f, 0.0f), 
    FORCE_DOWN (1.0f, 0.0f),
    // ... 
    IMP_UP (0.0f, 1.0f);

    private int f1,f2;

    private Force(float f1, float f2) {
      this.f1 = f1;
      this.f2 = f2;
    }
    public void getF1 { return f1; }
    public void getF2 { return f2; }
}

Answer 4

Looks like you're stuck in pre-1.5 Java :) Read about Enum's, EnumSet and a bit about Java Practices and the answer will be obvious to you.

Answer 5

You should read about Enums. This is clearly something that should be in an Enum class.

Answer 6

You can achieve better with enums and abstract methods (and good design, in general). It's hard to provide concrete guidance with the specifics of what you are trying to do. Here is an example of the type of code you might write:

public class Physics {
    public void apply(final Body body, final Vect place, final Direction dir) {
    float magnitude = findMagnitude();
    PhenomType phenomType = findPhenom();
    phenomType.apply(body, dir.createVect(magnitude), place);
    System.out.println("CAR Applied force: " + dir);
    }

    public enum Direction {
    UP {
        @Override
        public Vect createVect(final float magnitude) {
        return new Vect(0.0f, magnitude);
        }
    },
    DOWN {
        @Override
        public Vect createVect(final float magnitude) {
        return new Vect(0.0f, -magnitude);
        }
    },
    LEFT {
        @Override
        public Vect createVect(final float magnitude) {
        return new Vect(-magnitude, 0.0f);
        }
    },
    RIGHT {
        @Override
        public Vect createVect(final float magnitude) {
        return new Vect(magnitude, 0.0f);
        }
    };

    public abstract Vect createVect(float magnitude);
    }

    public enum PhenomType {
    FORCE {
        @Override
        public void apply(final Body body, final Vect vect, final Vect place) {
        body.applyForce(vect, place);

        }
    },
    IMPULSE {
        @Override
        public void apply(final Body body, final Vect vect, final Vect place) {
        body.applyImpulse(vect, place);
        }
    };

    public abstract void apply(Body body, Vect vect, Vect place);
    }
}

Answer 7

Vector2 vect = new Vector2();

could be done in a default branch of that switch statement to keep the JVM from creating an empty Vector2 when one of the other conditions would be satisfied.

Answer 8

One-true-brace style is easier to read (IMHO), and more common in Java:

if (dir == FORCE_RIGHT || dir == FORCE_LEFT || dir == FORCE_DOWN || dir == FORCE_UP) { body.applyForce(vect, place); }

Standard indent is 4 spaces.

Answer 9

Put the assignment to place before the switch, and then combine what's in the following if statement with the switch statement instead of testing it all twice.

Answer 10

Other than sticking to one style with your braces:

if (condition) {

vs

if (condition)
{

There isn't much you can improve on. The rest is just personal preference.

Answer 11

Suggestions that you will get here will usually be biased and will end up in endless arguments. I would just install checkstyle, PMD, findBugs etc and work on every suggestion that these tools give me