HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Reading large amount of player properties from CSV

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
readingamountplayerpropertiescsvlargefrom

Problem

I have written a class with a function that reads out the CSV from the webpage and creates an object with it.

The class seems too big - it seems very unoptimized, prone to mistakes and it's not very readable. How can I clean this class up so that it isn't so large and obstructive?

```
public class Player
{
public string Name { get; private set; }
public int Rank { get; private set; }
public int TotalLevel { get; private set; }
public int TotalExperience { get; private set; }
public int AttackRank { get; private set; }
public int AttackLevel { get; private set; }
public int AttackExperience { get; private set; }
public int DefenceRank { get; private set; }
public int DefenceLevel { get; private set; }
public int DefenceExperience { get; private set; }
public int StrengthRank { get; private set; }
public int StrengthLevel { get; private set; }
public int StrengthExperience { get; private set; }
public int ConstitutionRank { get; private set; }
public int ConstitutionLevel { get; private set; }
public int ConstitutionExperience { get; private set; }
public int RangedRank { get; private set; }
public int RangedLevel { get; private set; }
public int RangedExperience { get; private set; }
public int PrayerRank { get; private set; }
public int PrayerLevel { get; private set; }
public int PrayerExperience { get; private set; }
public int MagicRank { get; private set; }
public int MagicLevel { get; private set; }
public int MagicExperience { get; private set; }
public int CookingRank { get; private set; }
public int CookingLevel { get; private set; }
public int CookingExperience { get; private set; }
public int WoodcuttingRank { get; private set; }
public int WoodcuttingLevel { get; private set; }
public int WoodcuttingExperience { get; private set; }
public int FletchingRank { get; private set; }
public int FletchingLevel { get; private set;

Solution

The first thing I would do is break that up somehow.

Define a struct to encapsulate the three common properties between each stat:

public struct PlayerStat
{
    public int Rank { get; }
    public int Level { get; }
    public int Experience { get; }

    public PlayerStat(int rank, int level, int experience)
    {
        Rank = rank;
        Level = level;
        Experience = experience;
    }
}


Then this (and I'm only using these few lines as an example):

public string Name { get; private set; }
public int Rank { get; private set; }
public int TotalLevel { get; private set; }
public int TotalExperience { get; private set; }
public int AttackRank { get; private set; }
public int AttackLevel { get; private set; }
public int AttackExperience { get; private set; }
public int DefenceRank { get; private set; }
public int DefenceLevel { get; private set; }
public int DefenceExperience { get; private set; }


Goes down to:

public string Name { get; private set; }
public PlayerStat Total { get; private set; }
public PlayerStat Attack { get; private set; }
public PlayerStat Defense { get; private set; }


Then we'll take your parsing:

public Player GetHiscore(string name)
{
    WebRequest _request = HttpWebRequest.Create(string.Format("http://services.runescape.com/m=hiscore/index_lite.ws?player={0}", name));
    _request.Proxy = null;
    _request.AuthenticationLevel = AuthenticationLevel.None;

    using (WebResponse _response = (HttpWebResponse)_request.GetResponse())
    {
        using (StreamReader _reader = new StreamReader(_response.GetResponseStream(), Encoding.UTF8, false))
        {
            using (CsvReader csv = new CsvReader(_reader))
            {
                int Rank = Convert.ToInt32(csv.GetField(1));
                int TotalLevel = Convert.ToInt32(csv.GetField(2));
                int TotalExperience = Convert.ToInt32(csv.GetField(3));
                int AttackRank = Convert.ToInt32(csv.GetField(4));
                int AttackLevel = Convert.ToInt32(csv.GetField(5));
                int AttackExperience = Convert.ToInt32(csv.GetField(6));
                int DefenceRank = Convert.ToInt32(csv.GetField(7));
                int DefenceLevel = Convert.ToInt32(csv.GetField(8));
                int DefenceExperience = Convert.ToInt32(csv.GetField(9));


And clean it up a bit. First, remove the curly braces around the first two using statements, you have no code specific to them so we can omit those braces. These statements will then go to the same indentation level.

using (WebResponse _response = (HttpWebResponse)_request.GetResponse())
using (StreamReader _reader = new StreamReader(_response.GetResponseStream(), Encoding.UTF8, false))
using (CsvReader csv = new CsvReader(_reader))
{


Much better already, arrow-code removed. We can also use var instead of those types (for the most part):

using (var _response = (HttpWebResponse)_request.GetResponse())
using (var _reader = new StreamReader(_response.GetResponseStream(), Encoding.UTF8, false))
using (var csv = new CsvReader(_reader))
{


Let's make an extension method for csv:

public static class CsvExtensions
{
    public static T GetField(this CsvReader reader, int position)
        where T : IConvertible =>
        (T)Convert.ChangeType(reader.GetField(position), typeof(T));
}


So now we go from that block we just had for parsing to:

int Rank = csv.GetField(1);
int TotalLevel =csv.GetField(2);
int TotalExperience = csv.GetField(3);
int AttackRank = csv.GetField(4);
int AttackLevel = csv.GetField(5);
int AttackExperience = csv.GetField(6);
int DefenceRank = csv.GetField(7);
int DefenceLevel = csv.GetField(8);
int DefenceExperience = csv.GetField(9);


Simple, right? But we also have our struct so it gets even better:

Total = new PlayerStat(csv.GetField(1), csv.GetField(2), csv.GetField(3));
Attack = new PlayerStat(csv.GetField(4), csv.GetField(5), csv.GetField(6));
Defence = new PlayerStat(csv.GetField(7), csv.GetField(8), csv.GetField(9));


Good good, now we're down to much simpler work.

But wait, there's more.

We're going to take advantage of reflection and an array to clean this up another level.

First we'll define an array which is the order of all the properties in the CSV:

const string[] properties = new string[] { "Total", "Attack", "Defence", "Strength", ... }

for (int i = 0; i (i * 3 + 1), csv.GetField(i * 3 + 2), csv.GetField(i * 3 + 3));
    typeof(Player).InvokeMember(properties[i], BindingFlags.Public | BindingFlags.Instance | BindingFlags.SetProperty, Type.DefaultBinder, this, thisStat);
}


Then you just need to update your array.

Of course, all this means we have one more thing we can do: create an Attribute to encapsulate what properties of our class are in the CSV and where.

```
[AttributeUsage(AttributeTargets.Property)]
public class CsvAttribute : System.Attribute
{
public int StartPosition { get; set; }

public Cs

Code Snippets

public struct PlayerStat
{
    public int Rank { get; }
    public int Level { get; }
    public int Experience { get; }

    public PlayerStat(int rank, int level, int experience)
    {
        Rank = rank;
        Level = level;
        Experience = experience;
    }
}
public string Name { get; private set; }
public int Rank { get; private set; }
public int TotalLevel { get; private set; }
public int TotalExperience { get; private set; }
public int AttackRank { get; private set; }
public int AttackLevel { get; private set; }
public int AttackExperience { get; private set; }
public int DefenceRank { get; private set; }
public int DefenceLevel { get; private set; }
public int DefenceExperience { get; private set; }
public string Name { get; private set; }
public PlayerStat Total { get; private set; }
public PlayerStat Attack { get; private set; }
public PlayerStat Defense { get; private set; }
public Player GetHiscore(string name)
{
    WebRequest _request = HttpWebRequest.Create(string.Format("http://services.runescape.com/m=hiscore/index_lite.ws?player={0}", name));
    _request.Proxy = null;
    _request.AuthenticationLevel = AuthenticationLevel.None;

    using (WebResponse _response = (HttpWebResponse)_request.GetResponse())
    {
        using (StreamReader _reader = new StreamReader(_response.GetResponseStream(), Encoding.UTF8, false))
        {
            using (CsvReader csv = new CsvReader(_reader))
            {
                int Rank = Convert.ToInt32(csv.GetField(1));
                int TotalLevel = Convert.ToInt32(csv.GetField(2));
                int TotalExperience = Convert.ToInt32(csv.GetField(3));
                int AttackRank = Convert.ToInt32(csv.GetField(4));
                int AttackLevel = Convert.ToInt32(csv.GetField(5));
                int AttackExperience = Convert.ToInt32(csv.GetField(6));
                int DefenceRank = Convert.ToInt32(csv.GetField(7));
                int DefenceLevel = Convert.ToInt32(csv.GetField(8));
                int DefenceExperience = Convert.ToInt32(csv.GetField(9));
using (WebResponse _response = (HttpWebResponse)_request.GetResponse())
using (StreamReader _reader = new StreamReader(_response.GetResponseStream(), Encoding.UTF8, false))
using (CsvReader csv = new CsvReader(_reader))
{

Context

StackExchange Code Review Q#153236, answer score: 16

Revisions (0)

No revisions yet.