Можете оценить мой C# код?

Question

Этот код описывает человека и систему создания ID.

class Program
    {
        static void Main()
        {
            Person person1 = new Person("Николай", 19);
            Person person2 = new Person("Андрей", 25);
            Person person3 = new Person("Сергей", 23);
        Console.WriteLine("------------------------------");
        person1.PrintInfo();
        Console.WriteLine("------------------------------");
        person2.PrintInfo();
        Console.WriteLine("------------------------------");
        person3.PrintInfo();
        Console.WriteLine("------------------------------");

        //Поиск по известному ID и вывод информации
        Person.SearchByID(person2.ID).PrintInfo();

        Console.WriteLine("------------------------------");

        //Поиск по случайному ID (Несуществующему)
        Person.SearchByID("25462");
    }
}


public class Person : ID
    {
        static List<Person> persons = new List<Person>(); //Список созданных людей
    private string name;
    private int age;
    readonly public string ID;

    public string Name { get { return name; } set { name = value; } }
    public int Age { get { return age; } set { age = value; } }

    public Person(string name = &quot;Неизвестно&quot;, int age = 18)
    {
        ID = CreateID(); //Случайная генерация ID (ID не может повторяться)
        Name = name;
        Age = age;
        Console.WriteLine($&quot;Человек {name} создан&quot;);

        persons.Add(this);
    }

    public void PrintInfo()
    {
        Console.WriteLine($&quot;Имя: {name}\nВозраст: {age}\nID: {ID}&quot;);
    }

    public static Person SearchByID(string id)
    {
        Console.WriteLine($&quot;Поиск пользователя по ID {id}&quot;);
        for (int i = 0; i &lt; persons.Count; i++)
        {
            if (persons[i].ID == id)
            {
                Console.WriteLine(&quot;Результат найден!&quot;);
                return persons[i];
            }
        }

        Console.WriteLine(&quot;Человека с таким ID не существует!&quot;);

        return null;
    }
}


abstract public class ID
    {
        private List<string> ids = new List<string>(); //Список существующих ID
    protected string CreateID(int format = 4)
    {
        Random rnd = new Random();

        string id = &quot;&quot;;

        for (int i = 0; i &lt; format; i++)
        {
            id += rnd.Next(0, 10);
        }

        //Создание ID до тех пор, пока он не будет уникальным
        while (CheckID(id))
        {
            for (int i = 0; i &lt; format; i++)
            {
                id += rnd.Next(0, 10);
            }
        }

        ids.Add(id);

        return id;
    }

    //Проверка уникальности ID
    private bool CheckID(string id)
    {
        for (int i = 0; i &lt; ids.Count; i++)
        {
            if (ids[i] == id)
                return true;
        }

        return false;
    }
}

Можете написать о ошибках и недочётах этого кода, я был бы очень рад пополнить свои знания

Answer 1

Малая толика, по порядку строчек:

1. Для какой цели Person наследует ID?
2. Почему в 1 Person список persons? Это должно быть отдельно.
3. Поля и свойства, как вариант, пойдут. Хотя кодируется это проще

private string name;
public string Name { get { return name; } set { name = value; } }
-->
public string Name { get; set; }

4. Зачем в конструкторе такие значения по умолчанию? Как минимум, с какой-то начальной информацией объект должен инициализироваться. Возможно, нужны несколько конструкторов с разным набором параметров.
5. Что-то выводить в конструкторе в консоль - странно. Сделайте протоколирование.
6. Вместо PrintInfo можно переопределить стандартный метод ToString.
7. Поиск по идентификатору нужно не перебором делать, а использовать какой-то имеющийся метод. Опять же, никакого вывода в консоль.
8. Идентификатор нужно генерировать не случайный, а по порядку. И вообще, в чём в нём смысл в данной реализации? Можно просто использовать индекс в массиве или списке persons.

Тут уже можно код выбрасывать... Попробуйте переделать по этим замечаниям, для начала.

Answer 2

Из того, что не было замечено в других ответах:

1. Вывод на консоль не должен быть ответственностью модельного класса Person. Класс не должен знать, что его хотят вывести именно на консоль, а не, например, в Windows-приложении или в браузере.

2. Класс Person не должен являться наследником класса ID, так как он не является идентификатором, а всего лишь содержит его.

3. Хранение списка всех существующих ID не отскалируется на случай, если ваши данные хоть как-то распределены, или, например, вам понадобится сохранение списка на диск между запусками программы. Вам придётся делать сложные трюки, чтобы всё работало как надо.

4. Агрегация строки в цикле (for (int i = 0; i < format; i++) id += ... является антипаттерном. Используйте StringBuilder.

5. Для логирования используйте не вывод на консоль, а устоявшиеся механизмы логирования (не сейчас, а когда программа разрастётся до хотя бы 10 классов).