How can I optimize this code for better readability?

Question

I'm not a good programmer, for my bodypart-based collision detection in a game in unity I'm making I've ended up with a switch that looks like this despite my best attempts to simplify and shorten it:

public void GetCollision(Collision2D col) {

    if (attackType == -1) {
        if (col.gameObject.name == "Sword") {
            hitboxDisable();
        } else if (col.gameObject.name == "Player") {
            pim.PlayerDamage(5);
        }
    }

    if (col.gameObject.name == "Player_Body") {
        switch (attackType) {
            case -2: {
                    pim.PlayerDamage(5);
                }
                break;
            case 0:
                if (!pa.playerIsDodging) {
                    pim.PlayerDamage(5);
                } else {
                    pa.dodgeOnCooldown = false;
                    pa.resetDodgeRoutine();
                    hitboxDisable();
                }
                break;
            case 1:
                if (!swordSrc.isDefendingLegRight) {
                    pim.PlayerDamage(5);
                } else {
                    weaponBlocked = true;
                }
                break;
            case 2:
                if (!swordSrc.isDefendingArmRight) {
                    pim.PlayerDamage(5);
                } else {
                    weaponBlocked = true;
                }
                break;
            case 3:
                if (!swordSrc.isDefendingHeadRight) {
                    pim.PlayerDamage(5);
                } else {
                    weaponBlocked = true;
                }
                break;
            case 4:
                if (!swordSrc.isDefendingLegLeft) {
                    pim.PlayerDamage(5);
                } else {
                    weaponBlocked = true;
                }
                break;
            case 5:
                if (!swordSrc.isDefendingArmLeft) {
                    pim.PlayerDamage(5);
                } else {
                    weaponBlocked = true;
                }
                break;
            case 6:
                if (!swordSrc.isDefendingHeadLeft) {
                    pim.PlayerDamage(5);
                } else {
                    weaponBlocked = true;
                }
                break;
        }

        if (weaponBlocked == true) {
            hitboxDisable();
            RandomOpening();
            ApplyForce(testvar1, testvar2);
            weaponBlocked = false;
        }
    }
}

How can this be shortened and optimized for better readability? I'm new to C# and what little of my programming has been in C, I know there are a lot of ways to improve readability in C# I just don't know when/how to apply them. Suggestions would be much appreciated, I'm willing to try and learn anything, I want to try and avoid ending up with big switch statements like this if possible. Even just a suggestion what to apply here would be really appreciated, an example would be great.

I made the attackTypes into integers, they could have been strings but I chose not to because to my understanding strings take longer to compare. The attackType value specifies in the switch where the attack is targeting and if/how to block it, then if it was blocked.

This question is better suited for codereview.stackexchange.com and you will probably also get much better answers there. — Good Night Nerd Pride
– Good Night Nerd Pride, Commented Aug 16, 2022 at 8:13
Instead of the magic numbers consider using an enum. You could also do the same instead of swordSrc.isDefendingArmLeft, swordSrc.isDefendingHeadLeft, etc. and greatly reduce the repeated code. If the attack type matches the defense type you block, otherwise you take damage. — Retired Ninja
– Retired Ninja, Commented Aug 16, 2022 at 8:15
Okay, should I delete this and post it again there? Sorry I'm new here. — ShrikeDev
– ShrikeDev, Commented Aug 16, 2022 at 8:16
No worries. Don't bother deleting it, the mods will that for you :D — Good Night Nerd Pride
– Good Night Nerd Pride, Commented Aug 16, 2022 at 8:17
I’m voting to close this question because this question is better suited for codereview.stackexchange.com. — Guru Stron
– Guru Stron, Commented Aug 16, 2022 at 8:31

Kenten Fina · Accepted Answer · 2022-08-16 08:19:16Z

Case 1-6 seem very similar. You can make use of a local function

public void GetCollision(Collision2D col) {
    // this is a local function which you can only use inside this function
    // which can be useful if you have a short repeating pattern inside the function
    // and don't need it anywhere else
    void _handleHit(bool isDefending) {
        if (isDefending) {
            pim.PlayerDamage(5);
        } else {
            weaponBlocked = true;
        }
    }

    [...] // your code

    switch (attackType) {
        [...] // your code
        case 1: _handleHit(!swordSrc.isDefendingLegRight); break;
        case 2: _handleHit(!swordSrc.isDefendingArmRight); break;
        case 3: _handleHit(!swordSrc.isDefendingHeadRight); break;
        ...
    }
}

You could also take a look at C# enums and replace attackType with a readable version.

// Declaring the enum
public enum AttackType { Direct, LeftArm, ... }

// Then in a switch case you can do:
switch (attackType) {
    case AttackType.Direct: ...
    case AttackType.LeftArm: ...
}

JonasH · Accepted Answer · 2022-08-16 08:25:11Z

1

One thing would be to create a enum to describe the bodyparts, i.e.

public enum Bodypart{
None,
Head,
LeftArm,
RightArm,
LeftLeg,
RightLeg,
}

This could allow you to replace all the isDefendingArmLeft properties with a DefendingBodyPart. Likewise your attacks could be described by a combination of attack type and bodypart, this also removes the need to guess what attackType = -2 means:

public enum Attacktype{
Magic,
Sword,
Bodypart,
}

So that you can check first if the attacker attacks a specific body-part, and if the target is defending that specific body-part. If you want to allow defense or attack of multiple bodyparts you could use a [Flags] enum with one bit per bodypart.

answered Aug 16, 2022 at 8:25

JonasH

38.5k2 gold badges16 silver badges31 bronze badges

Collectives™ on Stack Overflow

How can I optimize this code for better readability?

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related