Require help on understanding proper variable scope and amount of parameters.

[HiddenMonk]

I am a bit unsure as to what people generally find proper in regards to where you declare your variables as well as how many parameters you use.

Generally, I like to have most if not all of my variables placed on top in the class so that I can see them all. I would declare iterator variables for loops and such usually at the loop, as well as any variable that I know I will just need temporarily for 1 method to run at Awake() or something closer to its scope.

Let me give an example. Here is my OverlapCapsule method.

using UnityEngine;
using System.Collections;

public partial class ExtensionsPhysics
{
    static Collider[] colliderArray;
    static Collider[][] colliderArrays;

    public static Collider[] OverlapCapsule(Vector3 position, Vector3 directionUp, float height, float radius)
    {
        LayerMask mask = Physics.AllLayers;
        return OverlapCapsule(position, directionUp, height, radius, mask);
    }

    public static Collider[] OverlapCapsule(Vector3 position, Vector3 directionUp, float height, float radius, LayerMask mask)
    {
        float capsuleBottomTopDistanceToCenter = radius * (((height / 2) / radius) - 1);

        Vector3 capsuelBottomPosition = position - (directionUp * capsuleBottomTopDistanceToCenter);
        Vector3 capsuelTopPosition = position + (directionUp * capsuleBottomTopDistanceToCenter);
   
        if(!Physics.CheckCapsule(capsuelBottomPosition, capsuelTopPosition, radius, mask)) return null;

        int overlapSphereCount = Mathf.CeilToInt(((height / radius) - 1));
        float overlapShereMoveDistance = (capsuleBottomTopDistanceToCenter * 2) / overlapSphereCount;

        colliderArrays = new Collider[overlapSphereCount+1][];

        for(int i=0;i<colliderArrays.Length;i++)
        {
            colliderArrays[i] = Physics.OverlapSphere(capsuelBottomPosition, radius, mask);

            capsuelBottomPosition += (directionUp * overlapShereMoveDistance);
        }

        return CleanUpArrays();
    }

    static Collider[] CleanUpArrays()
    {
        //will put in remove duplicate method later

        colliderArray = new Collider[GetMultiArrayLength.GetJaggedArrayLength<Collider>(colliderArrays)];

        int count = 0;
        for(int i=0;i<colliderArrays.Length;i++)
        {
            for(int j=0;j<colliderArrays[i].Length;j++)
            {
                colliderArray[count] = colliderArrays[i][j];
                count++;
            }
        }

        return colliderArray;
    }
}

Usually when it comes to static classes I would declare the variable close to its scope because static scares me or something, but you can see on top I have declared 2 Collider arrays. I have done this because I have read that parameters should be avoided, so instead of declaring these collider arrays close to scope and then passing them as a parameter to the “CleanUpArray” method, I just declared them at the highest scope. Is this the proper way to do things?

(Side question - When I do colliderArray = new Collider[“number here”][ ] what happens to the old array? Is this now going to garbage collection? Wouldn’t avoiding garbage collection be best? Should I be clearing this array and reusing it? How does unity do it with their OverlapSphere and such? Whenever I have an array in a loop, I don’t want to declare it close to scope. I feel like having it declared at top and created in a start() function or something and then just using it, instead of recreating over and over, especially if it will be large.)

Here is another script which would give you a better example of how I do things.

using UnityEngine;
using System.Collections;

public class GetCastsCenterOnCollision
{
    float capsuleCastStartPointHitPointDistance;
    float bottomPointHitPointDistance;
    float topPointHitPointDistance;
    float startPointHitPointHeight;
    float capsuleCastHeight;
    float capsuleHeightRadiusDifference;
    float capsuleCastCenterHitPointOffsetDistance;

    float sphereCastStartPointHitPointDistance;

    Vector3 capsuleCastStartPointCenter;
    Vector3 storedCapsuleCastStartPointCenter;
    Vector3 capsuleCastBottomTopPointDirection;
    Vector3 startPointHitPointDirection;
    Vector3 capsuleCastEndPoint;
    Vector3 startPointHitPointHorizontalAlignedPoint;
    Vector3 capsuleCastCenter;
    Vector3 capsuleCastCenterFlatBottom;

    Vector3 sphereCastEndPoint;
    Vector3 sphereCastCenter;

    SphereLineIntersect sphereLineIntersect = new SphereLineIntersect();
    MyVector3CrossMethods myVector3CrossMethods = new MyVector3CrossMethods();

    public Vector3 GetCapsuleCastCenterOnCollision(Vector3 capsuleCastBottomPoint, Vector3 capsuleCastTopPoint, float capsuleCastRadius, Vector3 capsuleCastDirection, Vector3 capsuleCastHitPoint)
    {
        capsuleCastStartPointCenter = (capsuleCastBottomPoint / 2) + (capsuleCastTopPoint / 2);
        storedCapsuleCastStartPointCenter = capsuleCastStartPointCenter;

        capsuleCastBottomTopPointDirection = (capsuleCastBottomPoint - capsuleCastTopPoint).normalized;
        capsuleCastStartPointHitPointDistance = Vector3.Distance(capsuleCastStartPointCenter, capsuleCastHitPoint);
        bottomPointHitPointDistance = Vector3.Distance(capsuleCastBottomPoint, capsuleCastHitPoint);
        topPointHitPointDistance = Vector3.Distance(capsuleCastTopPoint, capsuleCastHitPoint);

        capsuleCastEndPoint = capsuleCastStartPointCenter + (capsuleCastDirection * capsuleCastStartPointHitPointDistance);

        startPointHitPointHorizontalAlignedPoint = myVector3CrossMethods.GetVector3CrossPosition(capsuleCastStartPointCenter, capsuleCastHitPoint, capsuleCastBottomTopPointDirection);
        startPointHitPointHeight = Vector3.Distance(startPointHitPointHorizontalAlignedPoint, capsuleCastHitPoint);

        capsuleCastHeight = Vector3.Distance(capsuleCastBottomPoint,capsuleCastTopPoint) + (capsuleCastRadius * 2);
        capsuleHeightRadiusDifference = (capsuleCastHeight / 2) - capsuleCastRadius;

        if(startPointHitPointHeight > capsuleHeightRadiusDifference)
        {
            if(bottomPointHitPointDistance < topPointHitPointDistance)
            {
                capsuleCastStartPointCenter += capsuleCastBottomTopPointDirection * capsuleHeightRadiusDifference;
                capsuleCastEndPoint += capsuleCastBottomTopPointDirection * capsuleHeightRadiusDifference;
            }else{
                capsuleCastStartPointCenter -= capsuleCastBottomTopPointDirection * capsuleHeightRadiusDifference;
                capsuleCastEndPoint -= capsuleCastBottomTopPointDirection * capsuleHeightRadiusDifference;
            }
   
        }else if(bottomPointHitPointDistance < topPointHitPointDistance)
        {
            capsuleCastStartPointCenter += capsuleCastBottomTopPointDirection * startPointHitPointHeight;
            capsuleCastEndPoint += capsuleCastBottomTopPointDirection * startPointHitPointHeight;
        }else{                                               
            capsuleCastStartPointCenter -= capsuleCastBottomTopPointDirection * startPointHitPointHeight;
            capsuleCastEndPoint -= capsuleCastBottomTopPointDirection * startPointHitPointHeight;
        }

        capsuleCastCenter = sphereLineIntersect.GetSphereLineIntersect(capsuleCastStartPointCenter, capsuleCastEndPoint, capsuleCastHitPoint, capsuleCastRadius);
        capsuleCastCenter = storedCapsuleCastStartPointCenter + capsuleCastCenter;

        return capsuleCastCenter;
    }

    public Vector3 GetSphereCastCenterOnCollision(Vector3 sphereCastOrigin, float sphereCastRadius, Vector3 sphereCastDirection, Vector3 sphereCastHitPoint)
    {
        sphereCastStartPointHitPointDistance = Vector3.Distance(sphereCastOrigin, sphereCastHitPoint);

        sphereCastEndPoint = sphereCastOrigin + (sphereCastDirection * sphereCastStartPointHitPointDistance);

        sphereCastCenter = sphereLineIntersect.GetSphereLineIntersect(sphereCastOrigin, sphereCastEndPoint, sphereCastHitPoint, sphereCastRadius);
        sphereCastCenter = sphereCastOrigin + sphereCastCenter;

        return sphereCastCenter;
    }
}

As you can see, I declared many variables on top.
When people say parameters are bad, do they mean parameters within the method, for example, a helper method within the class shouldnt have parameters? Or do they mean the actual method itself should not really have parameters. If so, then how would you give info to the methods to do stuff, especially if the info keeps changing every frame? I really don’t know what they mean by avoiding parameters. Plenty of unitys methods have many parameters. Should many parameters only be used for static methods or something?

Please let me know how you or most people may generally do things, as well as reasons why. What is it that I am doing might be wrong or cause trouble in the future?

I know the saying “premature optimization is the root of all evil”, the problem is that I don’t know enough to know if I am over optimizing.

edit - my end result for the OverlapCapsule method
OverlapCapsule

using UnityEngine;
using System.Collections;
using System.Collections.Generic;

public partial class ExtensionsPhysics
{
    public static Collider[] OverlapCapsule(Vector3 position, Vector3 directionUp, float height, float radius)
    {
        LayerMask mask = Physics.AllLayers;
        return OverlapCapsule(position, directionUp, height, radius, mask);
    }

    public static Collider[] OverlapCapsule(Vector3 position, Vector3 directionUp, float height, float radius, LayerMask mask)
    {
        float capsuleBottomTopDistanceToCenter = radius * (((height / 2) / radius) - 1);

        Vector3 capsuelBottomPosition = position - (directionUp * capsuleBottomTopDistanceToCenter);
        Vector3 capsuelTopPosition = position + (directionUp * capsuleBottomTopDistanceToCenter);
     
        if(!Physics.CheckCapsule(capsuelBottomPosition, capsuelTopPosition, radius, mask)) return null;

        int overlapSphereCount = Mathf.CeilToInt(((height / radius) - 1));
        float overlapShereMoveDistance = (capsuleBottomTopDistanceToCenter * 2) / overlapSphereCount;

        Collider[][] colliderArrays = new Collider[overlapSphereCount+1][];

        for(int i=0;i<colliderArrays.Length;i++)
        {
            colliderArrays[i] = Physics.OverlapSphere(capsuelBottomPosition, radius, mask);

            capsuelBottomPosition += (directionUp * overlapShereMoveDistance);
        }

        if(colliderArrays == null) return null;

        return CleanUpArrays(colliderArrays);
    }

    static Collider[] CleanUpArrays(Collider[][] colliderArraysParam)
    {
        List<int> instanceIdList = new List<int>();
        List<Collider> colliderList = new List<Collider>();

        for(int i=0;i<colliderArraysParam.Length;i++)
        {
            for(int j=0;j<colliderArraysParam[i].Length;j++)
            {
                if(!instanceIdList.Contains(colliderArraysParam[i][j].GetInstanceID()))
                {
                    instanceIdList.Add(colliderArraysParam[i][j].GetInstanceID());
                    colliderList.Add(colliderArraysParam[i][j]);
                }
            }
        }

        return colliderList.ToArray();
    }
}

[JoeStrout]

You’re definitely overthinking it. Your goal in writing code should be to make it as clear as possible. The #1 obstacle to high-quality code is that it’s complex; software are among the most complex artifacts of human civilization, and no human can keep all the details in mind at once. So, good programming is all about making the intent of the code clear, and hiding details as much as possible to reduce the amount of stuff you have to grasp at once when reading the code.

Accordingly, you already know everything you need to know to answer questions like this. You need only rephrase each one as: “Which (position/approach/convention) seems clearer to me?” Then do that.

The premature optimization quote arises exactly because performance-optimized code is often more complex, less clear, and harder to understand than simple code. And it’s evil for all the reasons above (plus the fact that such premature optimization usually has no appreciable performance benefit).

As for the “parameters are bad” idea, I’ve no idea where you heard that. It’s not true.

So. Quit worrying about garbage collection. Forget about “that’s what they say.” Stop going through contortions to avoid parameters, or to have more parameters, or to use properties vs. local variables, or vice versa. Just write your code so that it is as simple, clear, and straightforward as you can. Your chances that the code will actually work, and that your project may actually get completed one day, will go up dramatically, and you’ll be happier too.

[jtsmith1287]

@JoeStrout

You’re definitely overthinking it.

I…

And then…

But…

Well…

Ya what he said.

I would like to emphasize readability. First glance at your code and I got a head ache. Lots of side scrolling and long lines. I always say that code should read like a book. I should be able to just read line by line through your code and know exactly what the intentions of that code are.

[HiddenMonk]

Here is an example of where people say to avoid parameters
http://stackoverflow.com/questions/12431932/how-many-parameters-in-c-sharp-method-are-acceptable

In regards to my code readability…
Which of the 2 were easier to read? I think I prefer the first one. A reason it might be hard for you to read my code is due to my long variable names, which also make it so the lines are long requiring you to scroll. I prefer very descriptive variables, though I do notice some poorly named variables. For example, overlapSpherePositionDistance should really be something like capsuleBottomTopDistanceToCenter (edit - changed it). I made that class just before making this post, so I guess I didnt clean it up to well.

[Kiwasi]

@HiddenMonk

http://stackoverflow.com/questions/12431932/how-many-parameters-in-c-sharp-method-are-acceptable

All of the answers here are about readability and usability. Nothing about performance. So sure, use only a few parameters. Wrap massive parameter lists in a class of their own. But don’t avoid them entirely. 32 parameters is bad. 5 is fine.

[shaderop]

@HiddenMonk

Here is an example of where people say to avoid parameters

Note that said question is closed and marked as “not constructive.”

There’s am article by John Carmack on Gamasutra titled “Functional Programming in C/C++” where he touches on the subject of parameters and mutating taste. The following bit is particularly relevant:

@John Carmack

The process of refactoring towards purity generally involves disentangling computation from the environment it operates in, which almost invariably means more parameter passing. This seems a bit curious – greater verbosity in programming languages is broadly reviled, and functional programming is often associated with code size reduction.

[HiddenMonk]

I mainly worry about large variables such as arrays when it comes to scope.

For example - using instantiate and then destroy for a bullet is just two lines of code, and is very simple and easy to understand. However, people advise to use object pools instead because creating and destroying every frame can cause spikes, give the garbage collector more work, etc…
Isnt a prefab just an array of components? (or is the slowdown because of what happens in the backend to get these components to work together?)
So now in my first script, I have two arrays declared at the top, and then a new one is being initialized in the first method and the cleanuparray method every time the method is called which could be every frame. This array could have 20 colliders in it (or more). Should I be reusing this array in some way or maybe use a list? And I am declaring the arrays at top, but would it be slower to declare closer to scope or does the compiler move it anyways? And If I moved it closer to scope, I would need to then pass these arrays as a parameter to the cleanuparrays method, so would that be even slower if the arrays contained 100 colliders, or does the parameter just care about the array and not actually pass whats inside, so an array with 100 vs 1 wouldnt matter.

I know that you are saying readability and such… But that is no reason for me to just stay blind to how things work and how I may be able to avoid slowdowns. I dont want to write all my code a certain way and then later find out that if only I moved a variable in a different location, all would be faster.
Instantiate and destroy would seem cleaner to me then storing a bunch of my objects and creating a system for reusing them, but it doesnt seem better performance wise to me, so Ill use pools instead of instantiating and destroying every so and so frames.
Instantiating a new collider array seems simpler and cleaner to me than trying to reuse it, but it doesnt seem better performance wise.
This is my delema.
When do I know what to do? What would you do depending on the array size, how often its used, the array type, etc…
What does the built in overlapsphere method, or any of unitys built in methods, do when they return arrays of things. Do they just do what I did in my overlapcapsule class?

[JoeStrout]

Seriously, don’t worry about things like object pools and recycling until you have actually profiled your nearly-complete game, and found out exactly where the performance problems are. Your intuition about where the performance problems are, is almost certainly wrong. (That’s no criticism of you in particular; everybody’s intuition about performance problems is wrong. That’s why they invented profilers.)

Frankly, it sounds to me like you’re getting a lot of advice from other newbies. No seasoned programmer will advise you to complicate your code without profiling it first.

If you want to develop a better coding style, beyond simple clarity (which should always be #1), then focus on hiding implementation details. Whether your colliders array is being created/destroyed each time, or is being kept around and reused, ought to be an detail that only one or two methods within a class know or care about; the rest of the code in the project should have no way to tell (nor should it need to!). So, after you’ve profiled it and discovered that this particular array is causing some performance problem, you can go in and change just that little part of the code, without breaking anything else.

(Changing the design of existing code, without changing its function, is known as refactoring and it’s a core part of any software development process. New developers try to write the code once, in the “correct” and often over-general way right from the beginning; experienced developers focus on clarity, simplicity, and correctness first, and refactor as needed.)

Hiding implementation details is known as encapsulation, and is a good habit to get into because (1) it makes refactoring easier, and (2) it reduces the amount of stuff you have to think about when you’re using the class/method/whatever later. So, in other words, it dovetails nicely with the “keep it simple, keep it clear” mantra you should be focusing on anyway.

So when faced with a choice — should I do it this way or that way? — you should ask yourself first: How can I hide this decision from the rest of the code? And once you’ve found a good way to do that, then which choice you make really doesn’t matter. Do it the simplest, clearest way now, and because you’ve hidden it from the rest of the app, it’s easy to change later as needed.

Cheers,

• Joe

[Strategos]

@HiddenMonk

Instantiating a new collider array seems simpler and cleaner to me than trying to reuse it, but it doesnt seem better performance wise.
This is my delema.
When do I know what to do?

You decide what to do on a case by case basis. In performance critical areas you put the extra effort into pooling your colliders or objects.

Profile your project to find bottlenecks and take steps to resolve them.

Edit: Or what Joe said lol

[HiddenMonk]

@Strategos

Edit:…Or what Joe said lol

Hehe
I guess I am just trying to save time in the future by doing things right the first time, but this might actually cause me to waste more time making things over optimized when not needed.
Also, since I am a fairly new coder, I am still trying to find my style of coding (as well as a proper/clean/better performance style) so that I can have all my code be consistent.
If I had the unity profiler then I would be a lot less concerned, however I am asking all these questions because I don’t know if Ill be able to get the pro until after / if a game I create sells well. So I am doing things blindly and want to know what the proper way to do things are in order to avoid needing the profiler as much.

But that all being said, I would still like to know what unity does in regards to returning arrays such as in the overlapsphere method. Is it just creating a new array like I am in my method?

[JoeStrout]

@HiddenMonk

Hehe
I guess I am just trying to save time in the future by doing things right the first time, but this might actually cause me to waste more time making things over optimized when not needed.

Now you’re getting it!

But that all being said, I would still like to know what unity does in regards to returning arrays such as in the overlapsphere method. Is it just creating a new array like I am in my method?

Yep. It has to, since it hands ownership of that array off to the caller, who will do who-knows-what with it.

But note that only the array is being created by this process. The elements of the array are just references to existing Colliders. References are tiny and lightweight. So, yeah, there will be garbage collection of the array there, and if you were calling this 1000 times in a loop on every frame it might show up in a sufficiently detailed profile report. But in general, it’s not a big deal and you should focus your attention elsewhere (like, on actually finishing your game!).

[HiddenMonk]

@JoeStrout

…But in general, it’s not a big deal and you should focus your attention elsewhere (like, on actually finishing your game!).

aye aye sir! =)
Thank you (and everyone) for your help!

[Timelog]

For future reference as far as coding principles go, it might be interesting to read up on the S.O.L.I.D. principles of programming in a OO environment. Just, don’t try to do it all at ones until you understand all the separate principles.

(S)ingle responsibility principle: Vacation Rental Hacks
(O)pen-Closed principle: Vacation Rental Hacks
(L)iskov substitution principle: Vacation Rental Hacks
(I)nterface segregation principle: Vacation Rental Hacks
(D)ependency inversion principle: Vacation Rental Hacks

Following these principles properly in general causes you to program in a very clean and easy to maintain way, making your code less bug prone and making debugging easier. It also increases the clarity of your code, and in general increases the usability and extendibility of the code at the same time.

[KelsoMRK]

@HiddenMonk

I guess I am just trying to save time in the future by doing things right the first time

Let me know if you find a full-proof way to do that - because I sure as hell haven’t.

[HiddenMonk]

@KelsoMRK

Let me know if you find a full-proof way to do that - because I sure as hell haven’t.

Try doing things wrong…you’ll probably get that right the first time ^_~

and thank you timelog, Ill be sure to read up on that!