Eine Datei für ein Spiel analysieren

2119
Jamal

Ist dieser Code gut genug oder ist er stinkig?

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;
        }
    }
}
Antworten
16
Gibt es einen Grund, warum Sie die Länge zurück in eine Zeichenfolge konvertieren? Sean Lynch vor 9 Jahren 1
Sie sollten den Dateistream schließen und wahrscheinlich mit einer using-Anweisung deklarieren, damit er entsorgt wird. Sean Lynch vor 9 Jahren 0
Total, aber ich frage nach der Art und Weise, wie ich mich selbst parse und wie ich die Parameter übergebe. (Auf jeden Fall positiv!) vor 9 Jahren 0
Ahh, ich hatte den Rest auf meinem Handy nicht bemerkt, es hatte keine Scrollbars. Sean Lynch vor 9 Jahren 0

7 Antworten auf die Frage

24
doppelgreener

Sie haben viele unbeschreibliche Zahlen und Code-Wiederholungen, während Sie den Inhalt eines Felds abrufen. Sie können die Wiederholung eliminieren und diese Zahlen etwas aussagekräftiger machen, indem Sie eine einzige Methode einführen:

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;
}

Verwenden Sie es wie folgt:

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

Hierbei ist zu beachten, dass sich der Füllwert geändert hat. Sie müssen nicht länger die Länge des Feldnamens selbst angeben und können anschließend einfach die Anzahl der Junk-Zeichen angeben.

Länge der Auffüllung

Bei der Untersuchung Ihres Codes fiel mir eine Besonderheit auf - die Felder haben inkonsistente, magische Polsterungslängen:

  • gameID-Auffüllung: 2
  • Länge der Spiellänge: 3
  • mapId-Auffüllung: 3
  • maxNumPlayers-Auffüllung: 3
  • creationTime-Auffüllung: 2

Als Symptom für diese magischen Zahlen habe ich keine Ahnung, warum dies der Fall ist. Dies ist einer der vielen Gründe, um magische Zahlen wie die Pest zu vermeiden: Es ist schwierig, ihre Bedeutung zu verstehen. Ich vertraue darauf, dass Sie prüfen, ob unterschiedliche Auffülllängen erforderlich sind oder ob Sie für alle Felder eine konstante Auffüllung annehmen können.

Wenn für alle Felder ein konstanter Füllungsbetrag angenommen werden kann, können wir den Code noch ein wenig ändern, um Ihnen das Leben zu erleichtern. Diese Änderung umfasst zwei Schritte.

Geben Sie Ihrer LogParserKlasse zunächst ein privates Feld:

private const var defaultPadding = 2

Zweitens GetFieldContentkann zu diesem Zweck umgestaltet werden:

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;
}

Dann wird es einfacher, den Inhalt eines Feldes zu erhalten:

var maxPlayers = GetFieldContent(content, "maxNumPlayers", 2);

6
akmad
  1. Entfernen Sie den Code, der für das Abrufen des Inhalts der Protokolldatei verantwortlich ist. Wenn dies ein Typ ist, der nur für das Analysieren von Protokolldateien verantwortlich ist, dann ist dies alles, was er tun sollte. Denken Sie darüber nach, entweder den Dateiinhalt an diese Methode zu übergeben oder die Abhängigkeitseinspritzung zu verwenden, damit Sie den StreamReaderAufruf in Ihren Tests überlisten können. Sie haben Tests richtig?
  2. Geben Sie die entsprechenden Typen aus diesen Methoden zurück. GetGameLengthsoll ein int zurückgeben und Game.Lengthsollte ebenfalls ein int sein. Gleiches für Maximum Players und Game Date.
  3. Die Get-Methoden sollten privat oder geschützt sein, es sei denn, es gibt einen bestimmten Grund, sie intern zu machen.
  4. Wenn es eine begrenzte Anzahl von Karten gibt (und alle bekannt sind), sollten Sie die Verwendung einer Aufzählung anstelle einer Zeichenfolge in Betracht ziehen. Wenn eine Aufzählung nicht geeignet ist, können Sie einen GameMap(oder einen solchen Typ erstellen, um die Informationen, die Sie in Bezug auf eine Karte interessieren, einzukapseln) und diese stattdessen zurückzugeben.
  5. Jede 'Get'-Methode muss überprüfen, ob die Standortvariable auf etwas> = 0 gesetzt ist, wenn Sie nach dem angegebenen Text suchen.
  6. Weisen Sie hilfreiche Ausnahmen an, wenn die Protokolldatei nicht die erwarteten Daten enthält (oder die Daten nicht der erwartete Typ sind). Eine NullReferenceException ist keine hilfreiche Ausnahme.
  7. Es gibt hier zu viele magische Zahlen und Saiten. Verschiebe diese in Konstanten mit hilfreichen Namen.
  8. Fügen Sie Kommentare mit Beispielen für den Protokolldateitext hinzu, den Sie in jeder "Get" -Methode suchen. Dadurch wird die Wartung erheblich vereinfacht, da Sie wissen, welche Art von Text Sie analysieren wollten.
  9. Wickeln Sie den StreamReaderin einen Verwendungsblock ein.
  10. Überprüfen Sie, ob die Datei vorhanden ist, bevor Sie versuchen, sie zu laden.
Mein Grund für die Verwendung von string anstelle von ints für Length ist, dass ich damit nichts berechnen werde. Warum sollte ich es in eine Zeichenfolge umwandeln, wenn ich es im Browser ausstelle? Wenn man es einfach als String von Anfang an belässt, scheint die beste Vorgehensweise zu sein. Danke für die anderen Tipps. vor 9 Jahren 0
Für Punkt 3 meinen Sie nicht extern, da die Idee ist, es nur innerhalb dieser Klasse zugänglich zu machen? stimmte mit allen anderen Punkten überein. greatwolf vor 9 Jahren 0
@victor Die Methode accessibility ist auf internal gesetzt, darauf habe ich mich bezogen. akmad vor 9 Jahren 0
3
k3b

Sie haben im Grunde eine prozedurale Implementierung Ihres eigenen Mechanismus erstellt, um ein Spielobjekt in und aus einer Datei zu serialisieren, die in Ordnung ist, wenn Sie das Dateiformat vollständig kontrollieren möchten.

Haben Sie sich das Konzept der Dotnet-Serialisierung angesehen ? Dort können Sie sehen, wie deklarativer Stil den zu schreibenden Code minimieren kann, indem Sie Attribute an die Game-Klasse und / oder ihre Eigenschaften anhängen.

Es gibt Implementierungen für binary, xml und json

Ich denke nicht, dass es eine großartige Idee ist. Das Spiel wird sich sehr weiterentwickeln, und wenn sich die Klasse weiterentwickelt, wird der Serializer wahrscheinlich nicht in der Lage sein, es zu berechnen. Der benutzerdefinierte Deserializer ist robuster. DonkeyMaster vor 9 Jahren 0
DonkeyMaster: Wollen Sie damit sagen, dass Sie und ein benutzerdefinierter Serializer in der Lage sind, Protokolle zu deserialisieren, die von einer alten Version des Programms geschrieben werden? Ich denke, das könnte ein Gegenmodell sein. Wenn nicht anders können Sie einen benutzerdefinierten Xml-Serialser über Ihr eigenes Flat-File-Format in Betracht ziehen. In jedem Fall hat das Poster in einem anderen Kommentar gesagt, dass er das Dateiformat nicht kontrolliert, so dass diese Unannehmlichkeiten irrelevant sind Lyndon White vor 7 Jahren 0
2
frennky

Sie könnten auch ersetzen:

StreamReader reader = new StreamReader(@"D:\Games\Riot Games\League of Legends\air\logs\LolClient.20110121.213758.log");
var content = reader.ReadToEnd();

mit:

var content = File.ReadAllText(@"D:\Games...log");

ReadAllText Methode, wie MSDN sagt: Öffnet eine Textdatei, liest alle Zeilen der Datei in eine Zeichenfolge und schließt dann die Datei.

Welches ist sogar kürzer als usingBlock.

1
Homde

Sieht so aus, als würden Sie eine einfache Protokolldatei überflüssig und standardmässig verwalten. Wenn Sie das Log in binär gespeichert haben, könnte es vielleicht entschuldigt werden, aber wenn Sie mit Textdateien arbeiten, warum sollten Sie nicht mit XML arbeiten? Oder besser: Warum gehen Sie nicht mit XML-Fragmenten? Linq to XML bietet eine hervorragende Unterstützung für die Verwaltung dieser Art von Dateien, sowohl für das Erstellen als auch für das Analysieren und Abfragen.

Ich analysiere eine Protokolldatei, die mir völlig aus der Hand geht. Vertrauen Sie mir, wenn ich die Wahl hätte, würde ich Json analysieren. : P vor 9 Jahren 2
Basierend auf dem Dateipfad scheint Sergio Tapia die Logfiles von League of Legends zu analysieren, vermutlich die, die unmittelbar nach Beendigung eines Matches erzeugt wurden und die Ereignisse dieses Matches beschreiben. doppelgreener vor 9 Jahren 0
0
Eric Bréchemier

Ich würde die Methoden GetGameID (), GetGameLength (), GetGameMap () ... umbenennen, da sie wie Getter aussehen, die in beliebiger Reihenfolge aufgerufen werden können, während sie tatsächlich analysieren und in einer bestimmten Reihenfolge aufgerufen werden müssen. Das ist auf den ersten Blick verwirrend.

Ich empfehle, "Get" durch "Read" oder "Parse" zu ersetzen:

  • ReadGameID ()
  • ReadGameLength ()
  • ReadGameMap ()
  • ...
Sie können in beliebiger Reihenfolge beliebig oft aufgerufen werden. Dies wäre nicht der Fall, wenn die Methoden im "reader" -Datenstrom ablaufen würden, die gesamte Datei wird jedoch gleich nach dem Öffnen in "content" eingelesen. Da die Methoden mit einem String arbeiten, müssen sie nicht in einer bestimmten Reihenfolge gelesen werden. doppelgreener vor 9 Jahren 0
0
Mohammad Tayseer
  1. Sie sollten einen Kommentar mit dem Verweis auf das Protokollformat einfügen, falls vorhanden.
  2. Es gibt überhaupt keine Fehlerprüfung.
  3. Ihr Code enthält nichts, was die Struktur der Datei widerspiegelt. Wenn ich an deiner Stelle wäre, würde ich Regex verwenden.