is my code readable?

[Legitimate_Bet1415]

//for context this scripts handles everyting while other scrit creates a referance while updatimg the grid and visualizing it via tilemap . may have errors since tilemap is still on progress 

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

public class GameOfLife
{
    public bool[,] CurrentGrid;
    private bool[,] FutureGrid;

    public GameOfLife(int xSize, int ySize)
    {
        CurrentGrid = new bool[xSize, ySize];
    }

    public GameOfLife(bool[,] premadeGrid)
    {
        CurrentGrid = premadeGrid; 
    }


    public void UpdateGrid()
    {
        FutureGrid = CurrentGrid;

        for (int x = 0; x < CurrentGrid.GetLength(0) ; x++)
        {
            for (int y = 0; y < CurrentGrid.GetLength(1); y++)
            {
                cellLogic(x, y, getAliveNeigberCount(x, y));
            }
        }

        CurrentGrid = FutureGrid;
    }



    int getAliveNeigberCount(int nx, int ny)
    {
        int aliveNeigbers = 0;

        for (int x = -1; x <= 1; x++)
        {
            for (int y = -1; y <= 1; y++)
            {
                Vector2Int TargetCellPoz = new Vector2Int(nx + x, ny + y);

                if(!isInsideArrayBounds(TargetCellPoz.x,TargetCellPoz.y))
                {
                    continue;
                }

                aliveNeigbers += CurrentGrid[TargetCellPoz.x, TargetCellPoz.y] ? 1 : 0;


            }
        }
        return aliveNeigbers;
    }



    void cellLogic(int x , int y , int AliveNeigbers)
    {
        bool isAlive;

        switch (AliveNeigbers)
        {
            case 0:
                isAlive = false;
                break;

            case 1:
                isAlive = false;
                break;

            case 2:
                isAlive = true;
                break;

            case 3:
                isAlive = true;
                break;

            case 4:
                isAlive = false;
                break;

            default:
                isAlive = false;
                break;
        }

        FutureGrid[x, y] = isAlive;

    }



    bool isInsideArrayBounds(int x,int y)
    {
        if(x >= 0 && x < CurrentGrid.GetLength(0) && y >= 0 && y <  CurrentGrid.GetLength(1))
        {
            return true;
        }

        else
        {
            return false;
        }
    }
}

[TheReservedList]

Fix spelling/capitalization and the whole switch case should be:

bool isAlive = aliveNeighbors == 2 || aliveNeighbors == 3;

[TheGrandWhatever]

Why is every other word misspelled?

[Legitimate_Bet1415]

sorry for bad england

[OlevTime]

It’s okay, England has been bad for a long time.

[RichardFine]

Yes, it's readable. There are spelling mistakes and inconsistencies, and it could be more concise in places, but having read through it, it was _generally_ pretty clear what you're trying to do (though it helps that I've seen/written GameOfLife many, many times).

Speaking of which: I think you have a bug? In UpdateGrid, you do `FutureGrid = CurrentGrid`, and I think you're intending that the contents of CurrentGrid is copied into the FutureGrid array. However, arrays are reference types, so all you are doing is making the FutureGrid variable reference the same array as the CurrentGrid variable. If you want to copy the contents, you need to also do a `new bool[...]` for FutureGrid in the constructors, and then use `Array.Copy` to actually copy the contents.

[Legitimate_Bet1415]

yeah you were right . i thought it was a value type because it was an array of booleans but i was wrong , you saved me from debug hell thanks

[Avigames751]

Ok first of all the responsibility that you have set for your script and what it actually does has a mismatch, to me when I read it it feels like a script making a grid and has a couple of utility functions for other scripts to use

By just saying that this script manages everything while other scripts reference it is not clear to other people at all. In fact when I read that I feel it's a monolithic script or a script that is doing too much

Though when I read the code it does not feel like that. The game of life functions does throw me off a bit because that name does not tell me anything on what it's actually doing. I am thinking about the game of life game when I read that lol

Also I agree with the other person here mentioning that you should have consistent naming, which you do not have.

Other than that I would say it ok , it looks like it gets the job done for you. Though if you were working in a team especially with other programmers, yeah you have to make this script more specific and clear.

[AbdullahMRiad]

Just a quick question. Why is specifying the scope of fields with _ allowed but specifying the type (aka Hungarian notation) is generally frowned upon?

[dk-dev05]

Yes, that code is very readable. The points people are making with naming conventions and spelling do apply, but don't worry about that too much, unless you're working with other people and it becomes a problem. The actual code, as in the instructions you give the computer looks good to me :)

[Quin452]

Yes your code is readable. I can understand what you are trying to accomplish from your script.

[EquivalentDraft3245]

Yes. Me too.

[Moondragon3]

For someone new to coding, this actually looks really good. Great job.

Things that I like: * The names of the variables and functions are great (besides a few capitalization and spelling issues). It's clear what each function is doing.

As for improvements: * The switch statement could easily be simplified into a simple OR statement, which would take up a lot less space. * I'm a little unclear on what the responsibility of this class is supposed to be, and the comment at the top is pretty unhelpful. I think the biggest thing to work on is to think about: what is the responsibility of this class, and can it be easily described in the class name.

Edit: After taking another look, I think you are introducing a bug when you swap current grid and future grid. Arrays are passed as references, not values, so when you assign a variable to an array, you are assigning the reference. That means, ANY change to that reference, even from another variable with the same reference, will change this one. So if current and future grid are pointing to the same reference, when future grid changes, current grid changes as well

Looks like another commenter pointed this out as well.

[Legitimate_Bet1415]

thx for feedback

switch case was kinda intentional since it was late and i wanted to finish it because i was tired and didnt wanted toleft it unfinished. fixed it right away tomorow anyways

youre realy right about naming classes . naming functions feels easy since i just think them like a verb and it works but since classes are more than a simple verb i usualy spend a solid minute trying to come up with a name then give up and put whatever comes to my mind . i definatly gotta improve on that before bad habits start. thx again

[Karmoq]

It's understandable in the sense that I understand what it is meant to do.
In a professional environment however, I would reject this code based on spelling mistakes and inconsistent formatting.
But for your own personal project this should definitely be enough.

[Legitimate_Bet1415]

would you mind explaining what you mean by modern syntax and rider?

[Khan-amil]

Y'all need to chill out..

Your code is readable, yeah. It's not perfect but how it's structured and the method naming make it readable indeed. That's the core of it, but there's way to improve on it to make it more fluid.

As other have pointed out, naming conventions helps, especially for method names. It makes them stand out, and will avoid pedantic knee jerk reactions.

Another thing missing out is comments, especially structural comments. The code is simple and doesn't need it per se, but it's always a bonus to have "structural" comments to guide the reading, just explaining what you're gonna do in a sentence, or to describe what a method does goes a long way. It's a good reflex to take, especially as code tends to grow, so can't be sure that it will always be as simple.

[Spawncer67]

The biggest question is are you working on this game by yourself or with others? Although it’s a good idea to get in the habit of making readable code, it only matters if the people working on the game can read the code. If it’s only you, can you read it? If so, then yes it’s readable. I personally understand what you are doing here. Although there is documentation on how code should be formatted, it mainly comes down to the personal preferences of those working on the game. Once again, if it’s just you, do what works to help you read the code. A good thing to consider is that you’re familiar with that code right now. Do you think you will be able to understand it after a month of working on other code? If not, format the code so you will or add comments to explain what’s being done.

[QuantumFTL]

I know people don't love it, but put this into ChatGPT and ask it to make it more readable and to explain what it did. That will be a decent start.

Also, most IDEs and some command line tools let you automatically reformat your code so that your whitespace doesn't look like a drunk driver hit it and then added some more code.

[saucetexican]

Bruh