Replace loop in EnableObjectInPool

Replaced the EnableObjectInPool with:

Array.Find(pool, p => p.activeInHierarchy == false)?.SetActive(true);
1 Like

Good job

Why? It’s virtually the exact same code at the cost of arguably becoming a little less readable. Depending on who you ask of course.

This is EnableObjectInPool

void EnableObjectInPool()
{
    for(int i = 0; i < pool.Length; i++)
    {
        if(pool[i].activeInHierarchy == false)
        {
            pool[i].SetActive(true);
            return;
        }
    }
}

and this is Array.Find

public static T Find<T>(T[] array, Predicate<T> match)
{
    if( array == null)
    {
        throw new ArgumentNullException("array");
    }

    if( match == null)
    {
        throw new ArgumentNullException("match");
    }
    Contract.EndContractBlock();

    for(int i = 0 ; i < array.Length; i++)
    {
        if(match(array[i]))
        {
            return array[i];
        }
    }
    return default(T);
}

5 - 10 years ago I’d agree with you that having the loops written out in full would be more readable but, nowadays with almost every language having some form of lambda expressions it is just as readable either way. Most code scanning tools such as Sonar give a lower score the more levels of indirection (pairs of braces) you have in your code. My day job in is writing in Swift and if I wrote the full loop plus nested condition like this then It would get picked up on in code review.

1 Like

Fair enough. Code review software - in my opinion - does not necessarily ‘know’ what readable code is. I have seen several articles in the past showing that it favours quite unreadable code over readable code because it ‘doesn’t like braces’ - for instance.

To be fair, I write very ‘contracted’ code on a daily basis. But I also work with other professional developers that aren’t learning to code. So, the sample you’ve shown is fine and I doubt anyone in my team or company would give it a second look and give the comment I did in my original post.

However, there are a lot of students here that are learning to code and it is hard enough to just get the basics under wraps. Throwing in lambdas and ‘syntactic sugar’ at a stage where people struggle to do the basics just confuses the matter. I personally think it would be more beneficial at an early stage to write it all out with all the braces and extras and start to understand what you are doing before starting to contract it into one-liners. I don’t do that myself and often help out with a code snippet that doesn’t comply to what I’m saying here, but even I am still learning to help people better.

I’ve shifted from ‘readability’ to ‘don’t confuse the students’. Your code is fine and from other posts I’ve seen you clearly don’t need to be told that. I would suggest that if you post more of these, perhaps throw in an explanation of what it’s doing and why. If you want. Or don’t. It’s up to you :sunglasses:

1 Like

Privacy & Terms