Страницы

Поиск по вопросам

воскресенье, 15 декабря 2019 г.

Рефакторинг программы

#c_sharp #wpf #инспекция_кода


Я написал программу,которая рандомным образом выбирает глагол на русском языке из
текстового документа и показывает нам его. Задача пользователя состоит в том,что он
должен ввести в текстбоксы этот же глагол в 1,2,3 формах английского языка.
Собственно сам код:

namespace Verbs2
{
   public partial class MainWindow : Window
    {
        int randomNum = 0;
        int counterWin = 0;
        int counterLose = 0;
        bool ifChecked = false;
        public MainWindow()
        {
            InitializeComponent();
        }

        public class Verbs
        {

            /*класс создан для работы над глаголами и текстового файла*/

            string[] readText = File.ReadAllLines(@"C:\Users\Рустем\Desktop\проекты\Verbs2\Verbs2\verbs.txt",
                System.Text.Encoding.GetEncoding(1251));//считываем все строки в массив
            List res = new List();
            public Verbs()
            {
                foreach (var line in readText)//перебираем строки массива
                {
                    res.Add(line.Split('.'));//Каждую строку сплитим и помещаем в
список массивов.
                }
            }
            public string GetVerb(int a)//метод позволяет достать из листа глагол
на русском языке(в 3 столбце находятся русские глаголы)
            {
                return res[a][3];
            }
            public string GetVerb(int a, int b)
            {
                return res[a][b];
            }
            public bool CheckVerbs(string a, string b, string c, int d)//проверяем
соответствуют ли введенные данные,той строке что мы выбрали
            {
                if ((a.Equals(res[d][0])) && (b.Equals(res[d][1])) && (c.Equals(res[d][2])))
                    return true;
                else return false;
            }
        }

        public void SetRandom()
        {
            /*в нашем текстовом документе 68 глаголов,поэтому мы выбираем один рандомный
глагол из 68*/
            Random rnd = new Random();
            randomNum = rnd.Next(0, 68);
        }

        public int GetRandom()
        {
            return randomNum;
        }

        private void btnstart_Click(object sender, RoutedEventArgs e)
        {
            /*при нажатии на кнопку "начать" создаем объект класса verb, выбираем
рандомной глагол, вставляем его в lblverb2 и делаем доступной кнопку btncheck*/
            Verbs verb = new Verbs();
            SetRandom();
            lblverb2.Content = verb.GetVerb(GetRandom());
            btncheck.IsEnabled = true;
            counterWin = 0;
            counterLose = 0;
        }

        private void btncheck_Click(object sender, RoutedEventArgs e)
        {
            /* в лэйблы 4-6 вставляем глаголы на английском языке
             проверяем соответсвуют ли введенные в текстбоксы глаголы,выбранному глаголу
             делаем доступной кнопку btnnext*/
            Verbs verb = new Verbs();
            lblverb4.Content = verb.GetVerb(GetRandom(), 0);
            lblverb5.Content = verb.GetVerb(GetRandom(), 1);
            lblverb6.Content = verb.GetVerb(GetRandom(), 2);
            if (verb.CheckVerbs(textverb1.Text, textverb2.Text, textverb3.Text, GetRandom()))
            {
                lborder.BorderBrush = Brushes.Green;
                counterWin++;
                lblcw.Content = counterWin.ToString();
            }
            else
            {
                lborder.BorderBrush = Brushes.Red;
                counterLose++;
                lblcl.Content = counterLose.ToString();
            }
            btnnext.IsEnabled = true;
            btncheck.IsEnabled = false;
            ifChecked = true;
        }

        private void btnnext_Click(object sender, RoutedEventArgs e)
        {
            /*по сути повторяет действия кнопки btnstart,но при этом очищает лэйблы
и текстбоксы*/
            Verbs verb = new Verbs();
            SetRandom();
            lblverb2.Content = verb.GetVerb(GetRandom());
            btncheck.IsEnabled = true;
            if (ifChecked == false)
                counterLose++;
            lblcl.Content = counterLose.ToString();
            ifChecked = false;
            lblverb4.Content = "";
            lblverb5.Content = "";
            lblverb6.Content = "";
            textverb1.Text = "";
            textverb2.Text = "";
            textverb3.Text = "";
            lborder.BorderBrush = Brushes.White;
        }
    }
}




Код XAML:


    
        
            
            
            
            
            
        
        
            
            
            
            
            
            
        
        
        
            
                
                
                
            
        

        
        
         
        

        
        
        
        
        
        
        
        

        
        
        


        
        
        

        
        
        
    



Хотелось бы провести рефакторинг кода. Что в нем можно улучшить?
    


Ответы

Ответ 1



Начнем с самого основного - класса. Вместо использования двумерного массива, лучше вынести логику работы с глаголом в отдельный класс, например: public class Verb { public string Rus { get; set; } public string F1 { get; set; } public string F2 { get; set; } public string F3 { get; set; } } названия свойство можно и более содержательные придумать =) Хранить элементы можно в обычном массиве: Verb[] data; public MainWindow() { InitializeComponent(); data = File.ReadLines("data.txt") .Select(x => x.Split('.')) .Select(x => new Verb { F1 = x[0], F2 = x[1], F3 = x[2], Rus = x[3] }) .ToArray(); } Обратите внимание на использование относительного пути, вместо абсолютного. Вместо разбора строки можно использовать десериализацию в удобный формат. Методы GetRandom() И SetRandom избыточны. Кроме этого вы жестко задаете верхнюю границу. Можно использовать свойство .Length: Verb current; private void setNextVerb() { current = data[rnd.Next(data.Length)]; } private void btnstart_Click(object sender, RoutedEventArgs e) { setNextVerb(); btncheck.IsEnabled = true; counterWin = 0; counterLose = 0; } На мой взгляд эта кнопка вообще не нужна и такой код лучше поместить, например, в конструктор. В методе CheckVerbs вы используете конструкцию вида: if something return true; else return false; которая может быть сведена к return something; То есть можно написать сразу так: //проверяем соответствуют ли введенные данные,той строке что мы выбрали private bool CheckVerbs(string a, string b, string c, Verb verb) { return a.Equals(verb.F1) && b.Equals(verb.F2) && c.Equals(verb.F3); } Тогда ваши обработчики изменятся следующим образом: private void btncheck_Click(object sender, RoutedEventArgs e) { lblverb4.Content = current.F1; lblverb5.Content = current.F2; lblverb6.Content = current.F3; if (CheckVerbs(textverb1.Text, textverb2.Text, textverb3.Text, current)) { ... и private void btnnext_Click(object sender, RoutedEventArgs e) { setNextVerb(); lblverb2.Content = current.Rus; Что касается XAML разметки - она не гибкая. Попробуйте изменять размер окна и вы поймете о чем идет речь. При первой возможности напишу об этом подробнее. Лучшее решение - переписать код через MVVM. Не хочу приводить здесь такой вариант, так как будет намного лучше, если вы сделаете это самостоятельно.

Ответ 2



1)В кнопках много логики. Кнопки должны содержать минимум логики и по возможности дергать 1 метод, где и будет описана вся логика. 2)Почему бы не создать отдельный класс, который будет содержать 4 поля: Русский глагол, 1-форма, 2-форма,3- форма? Затем, этот класс оборачиваешь в еще 1 класс, и хранишь все в List и делаешь методы сравнения и т п. Этим ты заменишь работу с массивом в Verbs. Ты сможешь работать с конкретными полями, а не рукоблудить индексы двумерного массива. 3)Возможно, имеет смысл хранить глаголы не в TXT файле, а в XML и выполнять десериализацию.(Особенное, если будешь следовать совету 2).

Ответ 3



Вот это очень плохо, т.к. путь к файлу прибит к вашему рабочему каталогу string[] readText = File.ReadAllLines(@"C:\Users\Рустем\Desktop\проекты\Verbs2\Verbs2\verbs.txt", System.Text.Encoding.GetEncoding(1251); Да, и зачем кодировка 1251. У вас что, работа в Windows XP планируется? Все ж давно на юникод перешли! Создайте папку в вашем проекте Assets, затем прав.клав.клик по этой папке Add->Existing Item->и укажите ваш verbs.txt установите у него свойства Build Action в Embedded Resource. А далее его можно будет прочитать так (пример из моего проекта = читаю список стран) StrList = new List(); using (Stream stream = GetType().Assembly.GetManifestResourceStream("WpfEmbeddedResource.Assets.Countries.txt")) using (StreamReader reader = new StreamReader(stream)) { string input = null; while ((input = reader.ReadLine()) != null) { StrList.Add(input); } } WpfEmbeddedResource - это название проекта.

Комментариев нет:

Отправить комментарий