Is there a way to shorten this if statement?

Unity Engine
,
1 
    public void RequestPickupGun(){
        _gunSpawner.GetRandomGun(out Gun randomGun);
        if(randomGun == null){
            return;
        }
    }

I would like to know if I could somehow do something similar to this line of code

        if (collider.TryGetComponent<IDealDamage>(out IDealDamage iDealDamage)){
2

I hate compound lines because they’re hard to understand at a glance.

Compound lines are also where bugs hide out, little bugs like exclamation marks.

  • Declare the IDealDamage variable.

  • Do a GetComponent() to it

  • Check if it’s not null and act on it.

If you have more than one or two dots (.) in a single statement, you’re just being mean to yourself.

How to break down hairy lines of code:

http://plbm.com/?p=248

Break it up, practice social distancing in your code, one thing per line please.

“Programming is hard enough without making it harder for ourselves.” - angrypenguin on Unity3D forums

“Combining a bunch of stuff into one line always feels satisfying, but it’s always a PITA to debug.” - StarManta on the Unity3D forums

1 Like
3 
    public void RequestPickupGun(){
        _gunSpawner.GetRandomGun(out Gun randomGun);
        if(randomGun == null){
            return;
        }
        _gunSpawner.RemoveGun(randomGun);
        PickupGun(randomGun);
    }

the issue i have with it is that its aesthetically unpleasing to have a return line in the middle of a function like this.

4

There are legitimate arguments against doing an “early out.”

None of them have to do with aesthetics. :slight_smile:

Invert it and do it when you find it.

if (foob != null)
{
  foob.Do();
}
1 Like
5 
if (!randomGun) return;
3 Likes
6

Also this quote:

Kurt-Dekker on the Unity forums. :smile:

That’s also supported by Clean … uhm … one of the half dozen Clean Something books. Clean Code probably. One method call per line, please.

Some people misunderstand verbosity. Long lines are ALSO verbose, and in a very bose way!
(“böse” is german for mean ;))

3 Likes
7

I don’t see that anyone mentioned it but why the out parameter? Did you write the GunSpawner class? Seems like unnecessary effort from what I can see but GetRandomGun could return a Boolean if you wrote the code and you could just check that instead. RemoveGun? Is it stuck in the spawner?

2 Likes
8

The trick is to make the GetRandomGun method return a bool value indicating whether or not a reference to a non-null object was assigned to the randomGun parameter.

public bool TryGetRandomGun(out Gun randomGun)
{
    int gunCount = guns.Length;
    if(gunCount == 0)
    {
        randomGun = null;
        return false;
    }
   
    int gunIndex = Random.Range(0, gunCount)
    randomGun = guns[gunIndex];
    return true;
}

public void RequestPickupGun()
{
    if(_gunSpawner.TryGetRandomGun(out Gun randomGun))
    {
        randomGun.Pickup();
    }
}
3 Likes
9

If the gun reference has been assigned to the out randomGun (in the spawner) it may as well RemoveGun(randomGun) from the array as well not leaving it to the caller.