P
P
Philip Ginkel2021-09-23 22:31:09
code review
Philip Ginkel, 2021-09-23 22:31:09

Review code required. How to make menu logic correctly?

Hello, I just started learning programming and C# language. I would like to hear the opinion of more experienced people about the code below.

Got a job

Define the class Employee (employee), containing the fields:
Full name . employee, position, date of birth, salary, department, length of service, status.
Create methods that allow you to:
A) transfer an employee to a new position;
B) transfer an employee to another department;
B) change the experience;
D) to change the salary;
E) dismiss an employee, send him on a business trip, vacation,
sick leave, take him off sick leave, while changing the status of the employee. By
default, the status is "Working";
E) display information about a specific employee.


The teacher also asked me to make a menu for calling all these methods. He did not provide any specific conditions. Therefore, I made a little more options for each method to work.
The menu could only be implemented using nested switch case constructs. In fact, it is my biggest doubt. Could you suggest how to improve this code. I don’t pin hopes on the teacher, he will appreciate this one too.
The code itself is quite voluminous and does not fit here
Here is the link https://pastebin.com/P07bV3nb
Thanks in advance.

Answer the question

In order to leave comments, you need to log in

1 answer(s)
S
spaceatmoon, 2021-09-24
@spaceatmoon

From the obvious in my opinion:
1. There is no validation, but it is better not to let it be filled with arbitrary data at all. It is better to replace it with a dictionary where in the console instead of "Enter a position" it will be "Choose from the proposed options" and opposite each position its own number.

public void SetNewPos()
        {
            Console.Write($"Введите должность для {Fullname}\nТекущая: {Position}\nНовая: ");
            this.Position = Console.ReadLine();
        }

2. Same for statuses point 1.
3. Instead of simple types, use objects for validation
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; }      //Состояние

Instead of separate classes like StatusDict, you can make them enum, but validation is still needed here
public class A {
    
    public StatusDict StatusCode {get; set;}
  
    public enum StatusDict
    {
      Open = 1,
      Close
    }
    
    public A(int code)
    {
      StatusCode = (StatusDict)code;
    }
  }

4. Don't do it. This is a pointless operation. With declared types, there will be 0, "", false depending on the type. And in general, this overload creates a hole in the system, where an undefined employee with such values ​​\u200b\u200bis floating in the database.
public Employee()
        {
            this.Fullname = "0";
            this.Department = "0";
            this.Salary = 0;
            this.Expirience = 0;
            this.Position = "0";
            this.Birthdate = "0";
        }

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

Not very familiar with C #, perhaps the curator will tell you. I would replace it with goto and be done with it;
Repeat:
            var key = Console.ReadKey();
            if (key.KeyChar == '1' || key.KeyChar == '3')
            {
                return;
            }
            else
            {
                goto Repeat;
            }

7. Lots of boilerplate code.
8. There are no checks for going beyond the boundaries of arrays
9. When you enter such a string 1,2, the beaver throws an error. No validation.
Convert.ToInt32(Console.ReadLine());
10. Why are there obvious comments in the code? You need to comment on difficult things.
Tried to lighten up your menu. I can note that knowing the menu, you can immediately go down to the desired floor.
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;

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question