Задать вопрос

Требуется код ревью. Как правильно составить логику меню?

Здравствуйте, я совсем недавно начал изучать программирование и язык C#. Хотел бы услышать мнение более опытных людей по поводу кода ниже.

Получил задание

Определить класс Employee (сотрудник), содержащий поля:
Ф.И.О. сотрудника, должность, дата рождения, оклад, отдел, стаж, статус.
Создать методы, позволяющие:
А) переводить сотрудника на новую должность;
Б) переводить сотрудника в другой отдел;
В) менять стаж;
Г) менять оклад;
Д) увольнять сотрудника, отправлять в командировку, в отпуск, на
больничный, выводить с больничного, меняя при этом статус сотрудника. По
умолчанию статус имеет значение «Работает»;
Е) выводить информацию о конкретном сотруднике.


Преподаватель попросил так же сделать меню для вызова всех этих методов. Каких то конкретных условий он не озвучивал. Поэтому я сделал чуть больше вариантов для работы каждого метода.
Меню смог реализовать только с помощью вложенных конструкций switch case. Собственно в нем мои самые большие сомнения. Не могли бы вы подсказать как улучшить данный код. На преподавателя надежд не возлагаю, он оценит и такой.
Cам код достаточно объемный и не помещается сюда
Вот ссылка https://pastebin.com/P07bV3nb
Заранее спасибо.
  • Вопрос задан
  • 554 просмотра
Подписаться 5 Простой 14 комментариев
Пригласить эксперта
Ответы на вопрос 1
@spaceatmoon
Из очевидного на мой взгляд:
1. Нет валидации, а лучше вообще не давать заполнять произвольными данными. Лучше заменить на словарь где в консоли вместо "Введите должность" будет "Выберете из предложенных вариантов" и напротив каждой должности своё число.
public void SetNewPos()
        {
            Console.Write($"Введите должность для {Fullname}\nТекущая: {Position}\nНовая: ");
            this.Position = Console.ReadLine();
        }

2. Тоже самое для статусов пункт 1.
3. Вместо простых типов используй объекты для валидации
private BirthdateFormat Birthdate { get; set; }   //Дата рождения
        private FullnameRules Fullname { get; set; }    //ФИО
        private Department DepartmentCode  { get; set; }  //Отдел
        private CompanyPosition Position { get; set; }    // Должность
        private int Salary { get; set; }         //Зарплата
        private int Expirience { get; set; }     //Стаж
        private StatusDict Status { get; set; }      //Состояние

Вместо отдельных классов типа StatusDict можно их сделать enum, но тут всё равно нужна валидация
public class A {
		
		public StatusDict StatusCode {get; set;}
	
		public enum StatusDict
		{
			Open = 1,
			Close
		}
		
		public A(int code)
		{
			StatusCode = (StatusDict)code;
		}
	}

4. Не делай так. Это бессмысленная операция. При объявленных типах там и так будет 0, "", false в зависимости от типа. Да и вообще эта перегрузка создаёт дыру в системе, где неопределённый сотрудник с такими значениями плавает в базе.
public Employee()
        {
            this.Fullname = "0";
            this.Department = "0";
            this.Salary = 0;
            this.Expirience = 0;
            this.Position = "0";
            this.Birthdate = "0";
        }

5. Очень плохо.
//Бесконечный цикл для работы меню
            for (; ; )

Не сильно знаком с C#, возможно куратор подскажет. Я бы заменил на goto и дело с концом;
Repeat:
            var key = Console.ReadKey();
            if (key.KeyChar == '1' || key.KeyChar == '3')
            {
                return;
            }
            else
            {
                goto Repeat;
            }

7. Много шаблонного кода.
8. Нет проверок на выход за пределы границ массивов
9. При вводе такой строки 1,2,бобёр валится ошибка. Нет валидации.
Convert.ToInt32(Console.ReadLine());
10. Зачем в коде очевидные комментарии? Комментировать нужно сложные вещи.
Попробовал облегчить твоё меню. Могу отметить, что зная меню, можно спуститься сразу на нужный этаж.
Repeat:
            Console.Clear();     //Очищает консоль после отработки функции
            string input;
            string currentLevel;
            Console.WriteLine("Выберите функцию");
            Console.WriteLine("1 Получить информацию");
            Console.WriteLine("2 Изменить должность");
            Console.WriteLine("3 Измеить отдел");
            Console.WriteLine("4 Изменить стаж");
            Console.WriteLine("5 Изменить зарплату");
            Console.WriteLine("6 Изменить статус");
            Console.WriteLine("0 Выход");
            //Двух уровневое меню реализованное c помощью линейного switch
            currentLevel = "";
            Command:
            //input = Console.ReadLine();
            currentLevel += Console.ReadLine();
            Console.WriteLine(currentLevel);
            switch (currentLevel)
            {
                case "1":
                    Console.WriteLine("Выберите количество \n1 Все \n2 Один \n3 Несколько");
                    goto Command;
                case "11":
                    for (int i = 0; i < Employers.Length; i++)
                    {
                        Employers[i].PrintInfo();
                        Console.WriteLine("___________________________");
                    }
                    Console.ReadKey();  //Костыль который не позволяет моментально очистить вывод
                    break;
                case "12":
                    int c12input;
                    for (int i = 0; i < Employers.Length; i++)
                    {
                        Console.Write(i + 1 + " ");
                        Employers[i].PrintName();
                    }
                    Console.WriteLine("Выберите сотрудника");
                    c12input = Convert.ToInt32(Console.ReadLine());
                    Employers[c12input - 1].PrintInfo();
                    Console.WriteLine("___________________________");
                    break;
                case "13":
                    Console.WriteLine("Выберите сотрудника");
                    Console.WriteLine("0 для возврата в предыдущее меню\n-------");
                    for (int i = 0; i < Employers.Length; i++)
                    {
                        Console.Write(i + 1 + " ");
                        Employers[i].PrintName();
                    }
                    Console.WriteLine("___________________________");
                    int c13input;
                    while ((c13input = Convert.ToInt32(Console.ReadLine())) != 0) //пока не введен 0, можно вводить индексы сотрудников
                    {
                            Employers[c13input - 1].PrintInfo();
                            Console.WriteLine("___________________________");
                    }
                    break;
           }
          goto Repeat;
Ответ написан
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы