C#•3w ago

✅ Code improvement

Hey guys, just wondering if there's a better way to do the function i did.

14 Replies

Mąż Zuzanny Harmider SzczęścieOP•3w ago

code.cs

Mąż Zuzanny Harmider SzczęścieOP•3w ago

if you want the code in some more readable space: https://paste.mod.gg/bgbysrsbzqco/0

BlazeBin - bgbysrsbzqco

A tool for sharing your source code with the world!

Angius•3w ago

foreach (var config in Config.Tiles.BaseGeneration)
{
    if (config.Value is ForegroundTileConfig fconfig)
    {
        if (fconfig.Max < tilesnapshot.Count(r => r.Tile.Type == config.Key))
        {
            if (config.Value.SpawnChance > Random.NextDouble())
            {
                var tl = tilesnapshot.OrderBy(r => Random.Next()).First();
                if (fconfig.SpawnableOn.Contains(tl.Tile.Type))
                {
                    if (tilesnapshot.Count(r => Tile.Collides(r.Tile, tl.Tile)) < 2)
                    {
                        await UpdateTileType(tl, config.Key);
                        transforms++;
                    }
                }
            }
        }
    }
}

foreach (var config in Config.Tiles.BaseGeneration)
{
    if (config.Value is ForegroundTileConfig fconfig)
    {
        if (fconfig.Max < tilesnapshot.Count(r => r.Tile.Type == config.Key))
        {
            if (config.Value.SpawnChance > Random.NextDouble())
            {
                var tl = tilesnapshot.OrderBy(r => Random.Next()).First();
                if (fconfig.SpawnableOn.Contains(tl.Tile.Type))
                {
                    if (tilesnapshot.Count(r => Tile.Collides(r.Tile, tl.Tile)) < 2)
                    {
                        await UpdateTileType(tl, config.Key);
                        transforms++;
                    }
                }
            }
        }
    }
}

could certainly be improved. Generally, it's a good idea to reduce nesting and "fail fast", so rather than

while(true)
{
  if (foo is 5)
  { 
    if (bar is 6)
    {
        DoStuff()
    }
  }
}

while(true)
{
  if (foo is 5)
  { 
    if (bar is 6)
    {
        DoStuff()
    }
  }
}

I would write

while(true)
{
    if (foo is not 5) continue;
    if (bar is not 6) continue;
    DoStuff();
}

while(true)
{
    if (foo is not 5) continue;
    if (bar is not 6) continue;
    DoStuff();
}

Mąż Zuzanny Harmider SzczęścieOP•3w ago

Angius•3w ago

tilesnapshot.OrderBy(r => Random.Next()).First()

tilesnapshot.OrderBy(r => Random.Next()).First()

could also be made more performant by using

tilesnapshot[Random.Next(0, tilesnapshot.Count)]

tilesnapshot[Random.Next(0, tilesnapshot.Count)]

Instead of sorting the whole collection just get the item at random index Besides that, it seems fine I'm not familiar with Unity enough to tell whether stuff like using the lock instead of some concurrent collection is a good choice

Mąż Zuzanny Harmider SzczęścieOP•3w ago

not unity, wpf

Angius•3w ago

Ah, huh, that Random threw me off Unity has static methods on Random class, while in C# you need an instance, and static methods are on Random.Shared instead

Mąż Zuzanny Harmider SzczęścieOP•3w ago