I really don't know what is considered better or even good for the most part. My code varies and has evolved over the years as a developer and sometimes I think it looks cleaner, but it is never easy to edit or add something. I fight with myself when writing things like particle engines because I know the purpose of a particle engine is to be quick and efficient, but how much overhead is there really just by giving a reference of an emitter to a particle that is contained within it? Why should I write behaviors for particles as functions that return a value vs just writing a function that returns nothing but handles all of the behavior? I feel like with the latter I end up with code that is easier to modify, but it looks terrible. With the previous I end up with very clean code but is god awful to edit as I have one iteration loop that basically controls everything based on the function it is given.

Here is some more recent code I have written that I think would be considered "good" but I am really not sure. The gist of this code is that it will spawn an enemy in a pattern that right now is determined by the writer. I will eventually write functions that determine which shape should be spawned when and where, but right now it just will spawn a shape from the SpawnPolyGroup function with 5 enemies per line and 6 lines in the shape. So it makes a hexagon with 5 enemies per line. My question is, is it better to have the SpawnGroup function control the for loop and return values from each SpawnPatterns (being Vector2 as locations), or would it be better to have each function in the SpawnPatterns class have it's own iterative loop? Also, any other tips on my code for cleanliness and professionalism would be great. I am self taught and never had a tutor, but have read a lot about people being self taught writing code that is unprofessional, but I have never reached out for someone to look at my code aside from a few people so I am hoping that I can learn something to be more professional in the future in hopes to find a paying job as a programmer.

   public static class Spawner
    {
        public static bool isSpawning = false;
        public static float spawnTimer = 0;
        public static bool groupSpawning = false;

        public static Dictionary<Enemy, float> delayedSpawnDict = new Dictionary<Enemy, float>();
        public static float timeDelta = 0;

        //Arguments for currentPattern should be (int maxEnemies, int currentIteration, int radius, Vector2 PatternLocation)
        public static Func<int, int, Vector2, Vector2, Vector2> currentPattern = SpawnPatterns.Line;

        public static void Update(GameTime gameTime)
        {
            if (!isSpawning)
            { return; }

            if (timeDelta > 0)
            {
                timeDelta -= gameTime.ElapsedGameTime.Milliseconds;
            }

            if (timeDelta <= 0 && delayedSpawnDict.Count() > 0)
            {
                KeyValuePair<Enemy, float> pair = delayedSpawnDict.ElementAt(0);
                Enemies.enemies.Add(pair.Key);
                pair.Key.Spawn();
                timeDelta = pair.Value;
                delayedSpawnDict.Remove(pair.Key);
            }


            //If enemies alive is greater than max enemies then do not spawn
            if (Enemies.enemies.Count + delayedSpawnDict.Count() >= 1)
            { return; }

            //Spawn Individual
            if (spawnTimer >= 500)
            {
                SpawnPolyGroup(gameTime, new Vector2(800, 0), 5, 6);
                spawnTimer = 0;
            }
            spawnTimer += gameTime.ElapsedGameTime.Milliseconds;
        }

        public static void SpawnGroup(GameTime gameTime, Vector2 location, Vector2 mod1, int amount = 5, float delay = 100)
        {
            for (int i = 0; i < amount; i++)
            {
                Vector2 enemyLocation = currentPattern(amount, i, mod1, location);

                if (enemyLocation == default(Vector2))
                {
                    return;
                }

                Enemy enemy = new Enemy(15, Assets.hollowCircle, enemyLocation, EnemyUtilities.Follow, 500);

                if (delay > 0)
                {
                    delayedSpawnDict.Add(enemy, delay);
                }
                else
                {
                    Enemies.enemies.Add(enemy);
                }

            }
        }

        public static void SpawnPolyGroup(GameTime gameTime, Vector2 location, int amount = 5, int sides = 5)
        {
            float radians = AOneMath.AngleToRadian(360 / sides);

            float rotation = AOneMath.AngleToRadian(90 * (sides % 2));

            for (int i = 0; i < sides; i ++)
            {
                int distanceFromCenter = 200;

                float x1 = location.X + (float)Math.Cos(radians * i + rotation) * distanceFromCenter;
                float y1 = location.Y + (float)Math.Sin(radians * i + rotation) * distanceFromCenter;
                Vector2 p1 = new Vector2(x1, y1);

                float x2 = location.X + (float)Math.Cos(radians * (i + 1) + rotation) * distanceFromCenter;
                float y2 = location.Y + (float)Math.Sin(radians * (i + 1) + rotation) * distanceFromCenter;
                Vector2 p2 = new Vector2(x2, y2);

                SpawnGroup(gameTime, p2, p1, amount);
            }
        }
    }

    public static class SpawnPatterns
    {

        /// <param name="radius">Radius, Best to have X and Y the same</param>
        /// <param name="patternLocation">Center location of circle</param>
        /// <returns>Enemy location based on current iteration of max from radius to the patternLocation as the center</returns>
        public static Vector2 Circle(int max, int current, Vector2 radius, Vector2 patternLocation)
        {
            float radians = AOneMath.AngleToRadian(360/max);

            float x = (float)Math.Cos(radians * current);
            float y = (float)Math.Sin(radians * current);

            return new Vector2(patternLocation.X + (x * radius.X), patternLocation.Y + (y * radius.Y));
        }

        /// <param name="pointB">The end point where the last enemy should be in the line</param>
        /// <param name="pointA">The start point where the first enemy should be in the line</param>
        /// <returns>Enemy location based on current iteration of max from pointA to pointB</returns>
        public static Vector2 Line(int max, int current, Vector2 pointA, Vector2 pointB)
        {
            float x = MathHelper.Lerp(pointA.X, pointB.X, (float)current / (float)max);
            float y = MathHelper.Lerp(pointA.Y, pointB.Y, (float)current / (float)max);

            return new Vector2(x, y);
        }


        /// <param name="p1">Top left point of area to spawn randomly</param>
        /// <param name="p2">Bottom right point of area to spawn randomly</param>
        public static Vector2 Random(int max, int current, Vector2 p1, Vector2 p2)
        {
            float x = AOneMath.random.Next((int)p1.X, (int)p2.X);
            float y = AOneMath.random.Next((int)p1.Y, (int)p2.Y);

            return new Vector2(x, y);
        }
    }

The Spawner class probably shouldn't be static - it would make it difficult to deal with if you ever needed two separate spawners, so you might as well change it and get the pain out of the way now.

SpawnPatterns seems OK as a static class for now.

The MSDN recommendations for public variables in C# are to use PascalCase (capitalized first letter), and private variables camelCase (lowercase first letter).  Though these are coding conventions and you're free to choose whatever you want.

That Func is complicated enough that you might want to define your own delegate type for it - this lets you give each parameter a name which will be helpful for remembering what each parameter does.

For cumbersome types like the KeyValuePair<Enemy, float> you could use 'var' instead to shorten it.

The {return;} all on one line looks weird.  Just take the braces off and indent the 'return;', or put the { } on separate lines.  This is another coding convention choice to make.

Overall your code looks pretty good to me.

Thanks for the critique, I wasn't aware of the PascalCase for public variables convention but I am trying to follow the "standard" convention so that is good to know. I like the recommendations and will do this and continue to do this in the future. I'm am really glad to hear that this isn't terrible because like I said, it is more recent code that I think looks much better than most of my previous. 

Good code:

• does what it's meant to (i.e. meets the product specification)
• does what it claims to (i.e. has accurately descriptive names and comments)
• is easy to understand
• is easy to change and extend

And really these are 4 overlapping aspects rather than 4 separate ones.

With those thoughts in mind, here's how I would approach your code:

• ditch all the 'static' stuff. That implies there is, and can only ever be, one instance of those things. That is not "easy to change and extend". There's a lot of documentation about why singletons and globals are bad, and static variables are part of the same family.
• remove unused arguments like 'GameTime'. The presence of the argument implies that the function does something with the current time, but it does not. This makes it harder to understand.
• Make sure your comments match your code. A comment says "//Spawn Individual" but the code says "SpawnPolyGroup". Your code should do what it claims to, and here it doesn't.
• Add comments to your functions. You don't want to have to read the whole function to know what it does, and the function names are rarely descriptive enough. You want this code to be easy to understand months after you've written it.
• Rename SpawnPatterns to something more mathematical - there's nothing intrinsic to spawning in those functions, just mathematics. This means your code isn't really doing what it claims (you think you're getting spawn patterns, but actually you just get basic geometry) and it's harder to extend (because if you find yourself needing basic geometry in future, you have to remember that you have some hidden away inside a SpawnPatterns class).
• Be consistent. You have a timeDelta which ticks downwards, and a spawnTimer which ticks upwards. It's not immediately clear why they differ and it's certainly not necessary to have them counting in different directions. As such, it's error-prone (as you might change the code and forget which one counts up and which one counts down) and hard to understand (what do these 2 different timers mean?) and not so easy to extend (could you perhaps have a standard class for timers like this?)

I see what you are saying about having params that aren't used but what would be a clean solution to that? The idea was to have the generic Func that i just call through the SpawnGroup function and i can easily change which function is called, either random, line, or circle, but this does end up having functions that use the paremeters differently as well as not using some at all. 

If you truly want a set of generic parameters then you might consider wrapping them in a struct, passing that to each of these functions instead, and providing comments/naming that makes it clear what you are doing, and why.

6 hours ago, Kylotan said:

Good code:

• does what it's meant to (i.e. meets the product specification)
• does what it claims to (i.e. has accurately descriptive names and comments)
• is easy to understand
• is easy to change and extend

And really these are 4 overlapping aspects rather than 4 separate ones.

With those thoughts in mind, here's how I would approach your code:

• ditch all the 'static' stuff. That implies there is, and can only ever be, one instance of those things. That is not "easy to change and extend". There's a lot of documentation about why singletons and globals are bad, and static variables are part of the same family.
• remove unused arguments like 'GameTime'. The presence of the argument implies that the function does something with the current time, but it does not. This makes it harder to understand.
• Make sure your comments match your code. A comment says "//Spawn Individual" but the code says "SpawnPolyGroup". Your code should do what it claims to, and here it doesn't.
• Add comments to your functions. You don't want to have to read the whole function to know what it does, and the function names are rarely descriptive enough. You want this code to be easy to understand months after you've written it.
• Rename SpawnPatterns to something more mathematical - there's nothing intrinsic to spawning in those functions, just mathematics. This means your code isn't really doing what it claims (you think you're getting spawn patterns, but actually you just get basic geometry) and it's harder to extend (because if you find yourself needing basic geometry in future, you have to remember that you have some hidden away inside a SpawnPatterns class).
• Be consistent. You have a timeDelta which ticks downwards, and a spawnTimer which ticks upwards. It's not immediately clear why they differ and it's certainly not necessary to have them counting in different directions. As such, it's error-prone (as you might change the code and forget which one counts up and which one counts down) and hard to understand (what do these 2 different timers mean?) and not so easy to extend (could you perhaps have a standard class for timers like this?)

Your explanation of good code outlines it really well for me. I will have to revisit a lot of code with these ideas in mind. I'm well on my way of practicing good habits and i appreciate the feedback

Do all those members really need to be public?  I'm a big believer in only exposing to the consumer of your code the bare minimum, and having them modify things through methods instead.  For example, what if in a later implementation, you find that you want to reset the spawnTimer when someone disables the spawner.  In your current implementation, that would be a painful retrofit, as you'd have to track down all the places where people are mucking with the internals of your class.  

Other things just seem unnecessary to expose, like timeDelta.  Why is that public?

And finally, whats up with all the hard coded numbers?   500?  800?  Without even a comment as to what they are.  Those should be named constants, or exposed so that users of the Spawner can set them to what they want.

36 minutes ago, Kylotan said:

If you truly want a set of generic parameters then you might consider wrapping them in a struct, passing that to each of these functions instead, and providing comments/naming that makes it clear what you are doing, and why.

I will have to look into that. I have never tried to use structs for filling parameters. You wouldnt have an example or be able to elaborate a little? This is a totally new idea to me, and i have never thought about using structs for anything really.