deleted 99 characters in body
Source Link
Jamal
  • 34.6k
  • 13
  • 129
  • 235

Overall, your code looks quite good. There is something however which you can simplify a lot.

Consider these two code-parts:

    if (!ninjaControl.isOnGround()) {
        airTime = airTime + tpf;
    } else {
        airTime = 0;
    }

    if (binding.equals("CharLeft")) {
        if (value) {
            left = true;
        } else {
            left = false;
        }
    } else if (binding.equals("CharRight")) {
        ...

All this follows the pattern if (condition) { value = a; } else { value = b; },
which

if (condition) { value = a; } else { value = b; }

which can be simplified using the Ternary operator to value = (condition ? a : b);

value = (condition ? a : b);

Using this operator, we can write:

    airTime = (!ninjaControl.isOnGround() ? airTime + tpf : 0);

    if (binding.equals("CharLeft")) {
        left = (value ? true : false);
    } else if (binding.equals("CharRight")) {
        ...

However, since we set left to true if value is true and to false if value is false, we are essentially setting it to the same value as value. So this series of if-else statements can be shortened to:

    if (binding.equals("CharLeft")) {
        left = value;
    } else if (binding.equals("CharRight")) {
        right = value;
    } else if (binding.equals("CharUp")) {
        up = value;
    } else if (binding.equals("CharDown")) {
        down = value;
    }

Besides this, my only suggestion would be to split up methods when they become too long, there are some of your methods that are on the edge of what I would call "too long" so this can be something to think about.

Overall, your code looks quite good. There is something however which you can simplify a lot

Consider these two code-parts:

    if (!ninjaControl.isOnGround()) {
        airTime = airTime + tpf;
    } else {
        airTime = 0;
    }

    if (binding.equals("CharLeft")) {
        if (value) {
            left = true;
        } else {
            left = false;
        }
    } else if (binding.equals("CharRight")) {
        ...

All this follows the pattern if (condition) { value = a; } else { value = b; },
which can be simplified using the Ternary operator to value = (condition ? a : b);

Using this operator we can write:

    airTime = (!ninjaControl.isOnGround() ? airTime + tpf : 0);

    if (binding.equals("CharLeft")) {
        left = (value ? true : false);
    } else if (binding.equals("CharRight")) {
        ...

However, since we set left to true if value is true and to false if value is false, we are essentially setting it to the same value as value. So this series of if-else statements can be shortened to:

    if (binding.equals("CharLeft")) {
        left = value;
    } else if (binding.equals("CharRight")) {
        right = value;
    } else if (binding.equals("CharUp")) {
        up = value;
    } else if (binding.equals("CharDown")) {
        down = value;
    }

Besides this, my only suggestion would be to split up methods when they become too long, there are some of your methods that are on the edge of what I would call "too long" so this can be something to think about.

Overall, your code looks quite good. There is something however which you can simplify a lot.

Consider these two code-parts:

if (!ninjaControl.isOnGround()) {
    airTime = airTime + tpf;
} else {
    airTime = 0;
}

if (binding.equals("CharLeft")) {
    if (value) {
        left = true;
    } else {
        left = false;
    }
} else if (binding.equals("CharRight")) {
    ...

All this follows the pattern

if (condition) { value = a; } else { value = b; }

which can be simplified using the Ternary operator to

value = (condition ? a : b);

Using this operator, we can write:

airTime = (!ninjaControl.isOnGround() ? airTime + tpf : 0);

if (binding.equals("CharLeft")) {
    left = (value ? true : false);
} else if (binding.equals("CharRight")) {
    ...

However, since we set left to true if value is true and to false if value is false, we are essentially setting it to the same value as value. So this series of if-else statements can be shortened to:

if (binding.equals("CharLeft")) {
    left = value;
} else if (binding.equals("CharRight")) {
    right = value;
} else if (binding.equals("CharUp")) {
    up = value;
} else if (binding.equals("CharDown")) {
    down = value;
}

Besides this, my only suggestion would be to split up methods when they become too long, there are some of your methods that are on the edge of what I would call "too long" so this can be something to think about.

Source Link
Simon Forsberg
  • 57.9k
  • 9
  • 149
  • 304

Overall, your code looks quite good. There is something however which you can simplify a lot

Consider these two code-parts:

    if (!ninjaControl.isOnGround()) {
        airTime = airTime + tpf;
    } else {
        airTime = 0;
    }

    if (binding.equals("CharLeft")) {
        if (value) {
            left = true;
        } else {
            left = false;
        }
    } else if (binding.equals("CharRight")) {
        ...

All this follows the pattern if (condition) { value = a; } else { value = b; },
which can be simplified using the Ternary operator to value = (condition ? a : b);

Using this operator we can write:

    airTime = (!ninjaControl.isOnGround() ? airTime + tpf : 0);

    if (binding.equals("CharLeft")) {
        left = (value ? true : false);
    } else if (binding.equals("CharRight")) {
        ...

However, since we set left to true if value is true and to false if value is false, we are essentially setting it to the same value as value. So this series of if-else statements can be shortened to:

    if (binding.equals("CharLeft")) {
        left = value;
    } else if (binding.equals("CharRight")) {
        right = value;
    } else if (binding.equals("CharUp")) {
        up = value;
    } else if (binding.equals("CharDown")) {
        down = value;
    }

Besides this, my only suggestion would be to split up methods when they become too long, there are some of your methods that are on the edge of what I would call "too long" so this can be something to think about.