Парсинг файла для игры


Этот код достаточно хорош, или это вонючка?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;

namespace DotNetLegends
{
    public class LogParser
    {
        /// <summary>
        /// Returns a populated Game objects that has a list of players and other information.
        /// </summary>
        /// <param name="pathToLog">Path to the .log file.</param>
        /// <returns>A Game object.</returns>
        public Game Parse(string pathToLog)
        {
            Game game = new Game();

            //On actual deployment of this code, I will use the pathToLog parameter.
            StreamReader reader = new StreamReader(@"D:\Games\Riot Games\League of Legends\air\logs\LolClient.20110121.213758.log");
            var content = reader.ReadToEnd();

            game.Id = GetGameID(content);
            game.Length = GetGameLength(content);
            game.Map = GetGameMap(content);
            game.MaximumPlayers = GetGameMaximumPlayers(content);
            game.Date = GetGameDate(content);

            return game;
        }

        internal string GetGameID(string content)
        {
            var location = content.IndexOf("gameId");
            var gameID = content.Substring(location + 8, 10);
            gameID = gameID.Trim();
            return gameID;
        }

        internal string GetGameLength(string content)
        {
            var location = content.IndexOf("gameLength");
            var gamelength = content.Substring(location + 13, 6);
            gamelength = gamelength.Trim();
            var time = Convert.ToInt32(gamelength) / 60;
            return time.ToString();
        }

        internal string GetGameMap(string content)
        {
            var location = content.IndexOf("mapId");
            var gameMap = content.Substring(location + 8, 1);
            switch (gameMap)
            {
                case "2":
                    return "Summoner's Rift";
                default:
                    return "nul";
            }
        }

        internal string GetGameMaximumPlayers(string content)
        {
            var location = content.IndexOf("maxNumPlayers");
            var maxPlayers = content.Substring(location + 16, 2);
            maxPlayers = maxPlayers.Trim();
            return maxPlayers;
        }

        internal string GetGameDate(string content)
        {
            var location = content.IndexOf("creationTime");
            var creationDate = content.Substring(location + 14, 34);
            creationDate = creationDate.Trim();
            return creationDate;
        }
    }
}


2110
16
задан 23 января 2011 в 02:01 Источник Поделиться
Комментарии
4 ответа

У вас есть много undescriptive магических чисел и повторение кода при извлечении содержимого поля. Вы могли бы устранить повторение и заставить эти цифры немного более осмысленным путем введения один способ:

protected string GetFieldContent(string content, string field,
int padding, int length)
{
var location = content.indexOf(field);
padding += field.Length;

var fieldVal = content.Substring(location + padding, length);
fieldVal = fieldVal.Trim();
return fieldVal;
}

Использовать его вот так:

internal string GetGameMaximumPlayers(string content)
{
var maxPlayers = GetFieldContent(content, "maxNumPlayers", 3, 2);
return maxPlayers;
}

Что-то здесь-стоимость обивка изменилось. Вам больше не нужно включать длину самого имени Поля и может только описать количество ненужных знаков после.

Длина подклада

Изучив код, я заметил одну особенность - поля несогласованности, волшебный длины подклада:


  • gameID обивка: 2

  • gameLength заполнения: 3

  • МАПИД заполнения: 3

  • maxNumPlayers заполнения: 3

  • creationTime обивка: 2

Как симптом этих магических чисел я не знаю, почему это так. Это одна из многих причин, чтобы избежать магических чисел, как от чумы: трудно понять их смысл. Я буду доверять вам, чтобы оценить, насколько разной длины прокладки необходимо, или же вы можете просто предполагают постоянное заполнение всех полей.

Если мы можем взять на себя постоянный обивка сумма для всех полей , то мы можем изменить код, еще немного, чтобы сделать вашу жизнь проще. Существуют два шага, чтобы это изменить.

Сначала, придать с помощью средства LogParser классе закрытое поле:

частная константный defaultPadding ВАР = 2

Во-вторых, GetFieldContent может быть переработан, чтобы произвести это:

protected string GetFieldContent(string content, string field, int length)
{
var location = content.indexOf(field);
var padding = defaultPadding + field.Length;

var fieldVal = content.Substring(location + padding, length);
fieldVal = fieldVal.Trim();
return fieldVal;
}

После получения содержимого поля становится проще:

ВАР максимальное количество игроков = GetFieldContent(контент, "maxNumPlayers", 2);

24
ответ дан 23 января 2011 в 09:01 Источник Поделиться


  1. Удалите код, отвечающий за извлечение содержимого лог-файла. Если это тип, который просто отвечает за парсинг лог-файлы, чем это все он должен делать. Подумайте, как передает в файл содержание этого метода или с помощью внедрения зависимостей, так что вы можете эмулировать то , что streamreader позвонить в тестах. У вас есть тесты, да?

  2. Возврат соответствующих типов из этих методов. GetGameLength должна возвращать int и игры.Длина должна быть int. Тоже самое для максимальных игроков и дата игры.

  3. Методы 'get' должны быть private или protected, если есть какая-то определенная причина, чтобы сделать их внутренними.

  4. Если есть ограниченное количество карт (а они все известны), вы можете хотеть рассматривать использовать перечисление, а не строка. Если перечисление не уместно вы можете создать GameMap (или какой-то такой тип инкапсуляции для вас информации касаемо карты) и возвращать ее.

  5. Каждый из вас должен способ проверить что переменная месте находится что-то >= 0, когда вы искать указанный текст.

  6. Полезно бросать исключения, если файл журнала не содержит ожидаемые данные (или данные не ожидается тип). Исключение NullReferenceException не исключение.

  7. Там слишком много магических чисел и строк здесь. Переместить их в константы с именами.

  8. Добавить комментарии с примерами файл журнала текст, который вы будете искать в каждом "получить" метод. Это делает обслуживание значительно проще, так как вы знаете, какой тип текста вы ожидали разбора.

  9. Оберните , что streamreader в блоке using.

  10. Проверьте, что файл существует, прежде чем пытаться загрузить его.

6
ответ дан 23 января 2011 в 05:01 Источник Поделиться

Кажется, ты делаешь много ненужных и стандарт управления простого журнала. Если у вас остались сохранения журнала в двоичном возможно, это можно простить, но видя, как вы работаете с текстовыми файлами, почему бы не пойти с XML? вернее, почему бы не пойти с фрагментами XML? LINQ для XML имеет отличную поддержку для управления этими видами файлов, создания, анализа и выполнения запросов.

1
ответ дан 23 января 2011 в 12:01 Источник Поделиться

Я бы переименовать GetGameID методов(), GetGameLength(), GetGameMap()... потому что они похожи на добытчиков, которые можно назвать в любом порядке, в то время как они выполняют на самом деле разбора и должны вызываться в определенном порядке. Это непонятное на первый взгляд.

Я предлагаю заменить 'вам' с 'читать' или 'парсить':


  • ReadGameID()

  • ReadGameLength()

  • ReadGameMap()

  • ...

0
ответ дан 23 января 2011 в 02:01 Источник Поделиться