Conditionally eliminating one of two colliding bodies

The first early return is fine.

if (!isPlayer(bodyA) || !isPlayer(bodyB)) { return; } 

This immediately tells the programmer that both bodies must be players.

The later ones less so, You can invert the condition and put the markForDestroy call in the then clause.

if (!save(bodyB)) markForDestroy(bodyB); 

I would add some blank lines to break up the function a bit though:

private void processContactBetweenPlayers(final Contact contact) { final Body bodyA = contact.getFixtureA().getBody(); final Body bodyB = contact.getFixtureB().getBody(); if (!isPlayer(bodyA) || !isPlayer(bodyB)) { return; } final float vx_a = Math.abs(bodyA.getLinearVelocity().x); final float vx_b = Math.abs(bodyB.getLinearVelocity().x); final float vy_a = Math.abs(bodyA.getLinearVelocity().y); final float vy_b = Math.abs(bodyB.getLinearVelocity().y); final float a_power = vx_a + vy_a; final float b_power = vx_b + vy_b; if ((a_power > b_power)) { if (!save(bodyB)) markForDestroy(bodyB); } else { if (!save(bodyA)) markForDestroy(bodyA); } } 

Here is how I would simply this:

First, I would create a reduced method for getting "body" in fixture as such:

contact.getBody("A"); 

Underneath the covers this could be

contact.getFixtureA().getBody() 

But by doing it this way you

1. Shorten the code
2. Create a type of factory method for these "fixtures". This may not be appropriate for your case but with no other context that is what I would do

And, what's with all the

final

Craziness? You only need to do this if you want to assure yourself (or the compiler) that it won't be reassigned. This is hyper defensive and, in 15 years of writing code I have never found it to be a necessity.

Frankly, 'final' in this case is repetitive and distracting. Unless you have a need for it (which, from this code, you don't) then ditch it.

this:

 if (!isPlayer(bodyA) || !isPlayer(bodyB)) { return; } 

is good, but this:

 if (!isPlayer(bodyA) || !isPlayer(bodyB)) { throw new IllegalArgumentException("Both bodies must be players!"); } 

Is way better! Think about it: In the previous case, you 'return' and terminate the function. But, in this condition you have no idea if the job was done or not and if not, why not. This is pre-condition checking and an ideal case for it

 final float vx_a = Math.abs(bodyA.getLinearVelocity().x); final float vx_b = Math.abs(bodyB.getLinearVelocity().x); 

Your variable names mean zilch to me. You HAVE to give them a better name. Also, create them up in groups of 2.

This code here is also problematic.

 if (save(bodyB)) return; markForDestroy(bodyB); 

The return statements are pointless here. You could just do:

 if (!save(bodyB)) markForDestroy(bodyB); 

And now, a line is gone and your code flows naturally and doesn't abruptly exit the function.

Finally, this thing where you put your

return 

on the same line is both cluttering and unnatural; They are sequential instructions to the CPU:

mv e248484 EAX cmp 0 JMP ... POPS 

Ok, yes that is 80686 assembly but you get the point. It's a branch and function return.

All, of course, my .02 USD

processContactBetweenPlayers is not a great name. A good name should describe what the method does, so the readers can make a good guess without studying the implementation. "process" is a wishy-washy term that doesn't mean anything.

This method also knows too much about the internal workings of a Contact object. By the law of Demeter, it would be good to avoid making chained references to contact.getFixtureA().getBody(). It would be better to pass two instances of Body to this method. Then the method could be renamed to:

private void collideBodies(Body bodyA, Body bodyB) { ... } 

---

Still about naming, it's a common practice to prefix method names that return a boolean value with is or has. Judging by your posted revised version, it would seem that isSaved would be a better name than save.

---

Since the calculation logic of the "power" is the same for both bodyA and bodyB and you have it repeated twice, it would be good to move this to a helper method. That would have the additional positive effect of hiding from this method the logic of calculating power, making it more cohesive, focusing on handling the collision, and not doing additional calculations.

---

In this condition you have some redundant parentheses:

 if ((a_power > b_power)) { 

I find it odd that the "power" is defined as the sum of the horizontal and vertical components of the velocity. I would expect it to be based on the speed, which is \$v = \sqrt{v_x^2 + v_y^2}\$. More specifically, I'd expect to compare the kinetic energies of the bodies, \$\frac{1}{2} mv^2\$.

Therefore, I would expect to see

if ((vx_a * vx_a + vy_a + vy_a) > (vx_b * vx_b + vy_b + vy_b)) { … } else { … } 

If you are instead modelling physics in some bizarro world where movement along the horizontal and vertical axes is considered less powerful than diagonal movement, then there should be an explanatory comment in the code.

After answers and comments I refactoring my code spliting the code in several methods.

private void processContact(Contact contact) { final Body bodyA = contact.getFixtureA().getBody(); final Body bodyB = contact.getFixtureB().getBody(); if (areBothPlayers(bodyA, bodyB)) { processContactBetweenPlayers(contact, bodyA, bodyB); } // others contacts processing. } private void processContactBetweenPlayers(Contact contact, Body playerBodyA, Body playerBodyB) { final float vx_a = Math.abs(playerBodyA.getLinearVelocity().x); final float vx_b = Math.abs(playerBodyB.getLinearVelocity().x); final float vy_a = Math.abs(playerBodyA.getLinearVelocity().y); final float vy_b = Math.abs(playerBodyB.getLinearVelocity().y); final float a_power = vx_a + vy_a; final float b_power = vx_b + vy_b; if (a_power > b_power) { if (!saved(playerBodyA)) { destroyIfNotSaved(playerBodyA); } } else { if (!saved(playerBodyB)) { destroyIfNotSaved(playerBodyB); } } } public void destroyIfNotSaved(Body playerBody) { if (!saved(playerBody)) { markForDestroy(playerBody); } } private boolean areBothPlayers(Body bodyA, Body bodyB) { return isPlayer(bodyA) && isPlayer(bodyB); }