Обратная связь по простым мелочам

Я начинающий программист-новичок, я начал немного зацикливаться на C # около двух месяцев назад, так как я начну изучение формального программирования примерно через 7 месяцев. Я слышал, как лучший способ узнать, как кодировать код, чтобы я написал это тривиальное приложение, и я хотел бы получить конструктивную обратную связь, какие хорошие вещи? И какие вещи я мог бы улучшить /сделал лучше.

Progam.cs

namespace EasyTrivia
{
    class Program
    {

        const bool DEBUG_MODE = false; 

        public static void DebugPrint(string printOut) {

            if (DEBUG_MODE) { Console.WriteLine("[DEBUG]: {0}", printOut); }
        }
        static void Main(string[] args)
        {
            //PLACEHOLDER, check for command line arguments
            for (int y = 0; y < args.Length; y++)
            {
                switch (args[y])
                {
                    case "debug1":
                        Console.WriteLine("DEBUG {0} ARGUMENT", y);
                        break;
                    case "debug2":
                        Console.WriteLine("DEBUG {0} ARGUMENT", y);
                        break;
                    case "debug3":
                        Console.WriteLine("DEBUG {0} ARGUMENT", y);
                        break;
                }
            }

            const int MAX_PLAYERS = 4; // TODO: Add more freedom here, so you can have any number of players

            string[] pNames = new string[4];
            Player[] player = new Player[4];

            Console.WriteLine("How many players wish to play? (max 4)");
            string input = Console.ReadLine();
            int noOfPlayers;
            bool parseInput = Int32.TryParse(input, out noOfPlayers); // We're doing the easy check

            while (noOfPlayers > MAX_PLAYERS || noOfPlayers < 1)
            {
                Console.WriteLine("Incorrect input. Please enter a valid number of players inbetween 1 and 4");
                input = Console.ReadLine();
                parseInput = Int32.TryParse(input, out noOfPlayers);
            } Console.Clear();

            for (int i = 0; i < noOfPlayers; i++)
            {
                Console.WriteLine("Please enter the name for player {0}", i + 1);
                pNames[i] = Console.ReadLine();
                player[i] = new Player();
                player[i].Name = pNames[i];
                player[i].Score = 0;

            } Console.Clear();

            GameLoop(player, noOfPlayers);
        }
        static void GameLoop(Player[] player, int noOfPlayers)
        {
           bool GameState = true;
           List<int> blacklist = new List<int>();
           Random RNG = new Random();
           const int WINNING_SCORE = 3; // TODO: Custom winning score should be configurable through commandline argument.
           TriviaHandler trivia = new TriviaHandler();
           trivia.InitializeQuestion();


            //Begin Game Loop
            while (GameState)
            {
                DebugPrint("Game loop initializing...");

                string questionString = trivia.FetchQuestion(blacklist, RNG);

                //If it's a duplicate entry FetchQuestion() will return null
                while (questionString == null) { questionString = trivia.FetchQuestion(blacklist, RNG); }

                Console.WriteLine(questionString);

                for (int i = 0; i < noOfPlayers; i++)
                {
                    Console.WriteLine("Player {0}, type in your answer:", i+1);
                    string pInput = Console.ReadLine();
                    if (trivia.CheckAnswer(pInput))
                    {
                        Console.WriteLine("Correct! You have been awarded 1 point");
                        player[i].Score += 1;
                        // If they get enough pts to win we gotta end the game here too.
                        if (player[i].Score == WINNING_SCORE) 
                        { 
                            Console.WriteLine("Congratulations player {0} has won the game! Would you like to play another one? (Y/N)", player[i].Name);
                            string choice = Console.ReadLine().ToUpper();
                            while (choice != "Y" && choice != "N")
                            {
                                Console.WriteLine("Incorrect input! Please press Y for yes or N for no");
                                choice = Console.ReadLine().ToUpper();
                            }
                            if (choice == "Y")
                            {
                                //Reset player score first
                                for (int z = 0; i < player.Length; i++)
                                {
                                    player[z].Score = 0;
                                }
                                GameLoop(player, noOfPlayers);
                            }
                            else { Environment.Exit(0); }
                        }
                        //If the answer is correct break the loop
                        break;
                    }
                    else { Console.WriteLine("Incorrect Answer, question is passed on to next player"); }
                } 
            }
        }
    }
}

Player.cs

namespace EasyTrivia
{
    class Player
    {
        private string name;
        public string Name { get; set; }

        private int score;
        public int Score { get; set; }
    }
}

TriviaHandler

namespace EasyTrivia
{
    class TriviaHandler
    {
        Question[] question = new Question[Question.GetMaxQuestions(true)];

        private int qID;
        public int QID { get; set; }

        public void InitializeQuestion()
        {
            List<string> questions = new List<string>(Question.ListA);
            //questions.AddRange(Question.ListA);
            List<string> answers = new List<string>();
            answers.AddRange(Question.ListB);

            Program.DebugPrint("Initializing questions...");
            for (int i = 0; i < answers.Count; i++)
            {
                question[i] = new Question();
                question[i].Pretext = questions[i];
                question[i].Answer = answers[i];
                //if (options) { question[i].LoadOptions() }
            }
        }

        public bool CheckAnswer(string input)
        {
            //if (input == "debug") { return true; }
            if (input.ToUpper() ==  question[QID].Answer.ToUpper()) { return true; }
            else { return false;  }
        }
        public string FetchQuestion(List<int> blacklist, Random RNG)
        {
            bool blisted = false;
            int QuestionID = RNG.Next(Question.counter);

            if (blacklist.Contains(QuestionID)) { blisted = true; }

            if (!blisted)
            {
                QID = QuestionID;
                blacklist.Add(QuestionID);
                return question[QuestionID].Pretext;
            }
            else { return null; }
        }
    } 
}

Question.cs

namespace EasyTrivia
{
    class Question
    {
        private string pretext;
        private string answer;
        private string[] options;
        private string[] listA;
        private string[] listB;

        public static string[] ListA { get; set; }
        public static string[] ListB { get; set; }
        public string Pretext { get; set; }
        public string Answer { get; set; }
        public static int counter { get; set; }

        public static int GetMaxQuestions(bool initialize)
        {
            int count = new int();
            List<string> questions = new List<string>();
            List<string> answers = new List<string>();
            Program.DebugPrint("Getting max questions...");
            XElement rawXML = XElement.Load(@"C:\Users\Gabriel\Documents\Visual Studio 2013\Projects\EasyTrivia\EasyTrivia\Questions.xml");
            IEnumerable<XElement> elements = rawXML.Elements();

            //So I've written three ways of fetching the Q/A data

            //Method 1
            //foreach (XElement question in elements)
            //{
            //    if (initialize) { count++; }
            //    questions.Add(question.Element("Pretext").Value);
            //    answers.Add(question.Element("Answer").Value);
            //}
            //if (initialize)
            //{
            //    ListA = questions.ToArray();
            //    ListB = answers.ToArray();
            //}

            //Method 2
            var pQuery = from lQ in elements.Elements()
                         where lQ.Name == "Pretext"
                         select lQ.Value;
            var aQuery = from aQ in elements.Elements()
                         where aQ.Name == "Answer"
                         select aQ.Value;

            foreach (var pretext in pQuery)
            {
                if (initialize) { count++; }
                questions.Add(pretext);
            }
            foreach (var answer in aQuery)
            {
                answers.Add(answer);
            }
            if (initialize)
            {
                ListA = questions.ToArray();
                ListB = answers.ToArray();
            }

            //Method 3
            //string[] lines = File.ReadAllLines("Questions.txt");
            //for (int i = 0; i < lines.Length; i++)
            //{
            //    if (lines[i].Contains("[QUESTION") == true) { count++; }

            //    if (initialize && lines[i].Contains("[QUESTION"))
            //    {
            //        string[] delimiter1 = lines[i + 1].Split('=');
            //        string[] delimiter2 = lines[i + 2].Split('=');
            //        questions.Add(delimiter1[1]);
            //        answers.Add(delimiter2[1]);
            //        ListA = questions.ToArray();
            //        ListB = answers.ToArray();

            //    }
            //}
            if (initialize) { counter = count; }
            return count;

        }
    }
}

Questions.xml

<?xml version="1.0" encoding="utf-8" ?>
<QUESTIONS>
  <Question id="1">
    <Pretext>This is the first test question</Pretext>
    <Answer>test1</Answer>
  </Question>
  <Question id="2">
    <Pretext>This is the second test question</Pretext>
    <Answer>test2</Answer>
  </Question>
  <Question id="3">
    <Pretext>This is the third test question</Pretext>
    <Answer>test3</Answer>
  </Question>
</QUESTIONS>
c#
11 голосов | спросил Overly Excessive 31 Jpm1000000pmFri, 31 Jan 2014 13:45:29 +040014 2014, 13:45:29

2 ответа


15

Первое примечание - не используйте константу DEBUG_MODE, чтобы определить, следует ли печатать отладочное сообщение или нет. Есть лучшие способы сделать это. Вы можете использовать условный атрибут , чтобы проверить, является ли символ компиляции " DEBUG ". Вызовы методов, отмеченных условным атрибутом, полностью удалены из сгенерированного кода, если символ не определен («DEBUG» определяется по умолчанию при запуске приложения в режиме отладки):

[Conditional("DEBUG")]
public static void DebugPrint(string printOut) 
{
    Console.WriteLine("[DEBUG]: {0}", printOut);
}

Другим вариантом является использование библиотеки журналов (NLog, log4net и т. д.). Вы сможете определить цели ведения журнала (или добавочные файлы), которые будут писать сообщения в консоль, файл, базу данных и т. Д. Вы сможете определять уровни протоколирования сообщений из файла конфигурации без изменения кода приложения. Например. с NLog вы можете получить экземпляр журнала

private static Logger Logger = LogManager.GetCurrentClassLogger();

И используйте его для написания отладочных сообщений, если сообщение об уровне отладки включено в конфигурации (вы можете изменить формат сообщения из конфигурации):

Logger.Debug("Initializing questions...");

Вернуться к приложению. Я предлагаю вам следовать стилям заглавной буквы для типов имен, методов и переменные, предложенные Microsoft. Например. локальные переменные должны иметь имена camelCase. Также не используйте префиксы и сокращения в именах переменных - pNames менее читабельны, чем playerNames. И трудно понять, что такое listA и listB, почему запросы имеют имена pQuery и aQuery и их имена переменных запроса lQ и aQ


Если вы используете автоматически реализованные свойства, вам не нужно определять поля:

class Player
{        
    public string Name { get; set; } // back-storage will be generated
    public int Score { get; set; }
}

Класс Question странный. Первое, что не очевидно, это то, что он инициализируется, когда вы проверяете вопросы max что-то. Я предлагаю вам создать класс Question, который будет представлять данные, которые у вас есть в xml:

public class Question
{
    public int Id { get; set; }
    public string Pretext { get; set; }
    public string Answer { get; set; }
}

Я предлагаю не записывать имена файлов жесткого кода в приложении. Переместить имя файла вопросов для перемещения в настройки приложения:

<appSettings>
   <add key="questionsFileName" 
        value="C:\EasyTrivia\EasyTrivia\Questions.xml"/>
</appSettings>   

И переместите логику доступа к данным в отдельный класс (с этим классом Question вы можете использовать атрибуты сериализации Xml для десериализации вопросов из XML-файла), например

public class QuestionRepository
{
    private static readonly string fileName;

    static QuestionRepository()
    {
        fileName = ConfigurationManager.AppSettings["questionsFileName"];
    }

    public List<Question> GetAllQuestions()
    {
        XDocument xdoc = XDocument.Load(fileName);
        var query = from q in xdoc.Root.Elements("Question")
                    select new Question {
                        Id = (int)q.Attribute("id"),
                        Pretext = (string)q.Element("Pretext"),
                        Answer = (string)q..Element("Answer")
                    };

        return query.ToList();
    }
}

Как я понимаю логику вашего blackList - вы не хотите получать один и тот же вопрос несколько раз, и вы хотите задать вопросы в случайный порядок. Чем можно сделать очень легко без каких-либо черных списков и циклов, а следующий случайный вопрос не в черном списке:

var repository = new QuestionRepositor();
var random = new Random();
var questions = repository.GetAllQuestions().OrderBy(q => random.Next());

Это будет анализировать xml, возвращать вопросы и сортировать их в произвольном порядке. Теперь ваш игровой цикл может выглядеть так:

foreach(var question in questions)
{
    Ask(question); // extract all loop code to some method
    if (players.Any(IsWinner))
        break;
}

Извлеченный код может выглядеть так:

private void Ask(Question question)
{
     Console.WriteLine(question.Pretext);
     foreach(var player in players) 
     {
         if (question.IsAnswerCorrect(GetAnswer()))
             player.Score++;

         if (IsWinner(player))
             return;
     }
}

private bool IsWinner(Player player)
{
     return player.Score == WinningScore;
}

У вашей программы есть другие улучшения, но ответ слишком велик для формата вопрос-ответ. Продолжайте и используйте рекомендации выше.

ответил Sergey Berezovskiy 31 Jpm1000000pmFri, 31 Jan 2014 14:44:09 +040014 2014, 14:44:09
6

Есть много хороших материалов от Сергея и Андриса, поэтому я сделаю свой короткий:)

  • Вам следует избегать повторного кода, как у вас в основном методе, с корпусом коммутатора.
  • Вам следует избегать больших методов, таких как GameLoops и GetMaxQuestions. Разбейте их на более мелкие методы.
  • Вы можете заменить некоторые из ваших циклов foreach петлями для простоты.

Одна вещь, которая жизненно важна для рефакторинга и тестируемости, заключается в том, что вы понимаете, что происходит внутри метода без фактического разделения содержимого. Чтобы добиться этого, используйте именование, чтобы уточнить и избежать таких переменных, как вопрос /вопросы, если они не находятся в цикле foreach. Переименование вопросов на что-то вроде allAvailableQuestions поможет вам не смешивать их и должно быть более ясным, что этот массив для

ответил Helge Heldre 31 Jpm1000000pmFri, 31 Jan 2014 15:57:19 +040014 2014, 15:57:19

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

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132