How can I optimize this code for better readability?-CodePudding

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.

CodePudding user response：

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: ...
}

CodePudding user response：

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.