6
\$\begingroup\$

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; }
    public int FletchingExperience { get; set; }
    public int FishingRank { get; set; }
    public int FishingLevel { get; set; }
    public int FishingExperience { get; set; }
    public int FiremakingRank { get; set; }
    public int FiremakingLevel { get; set; }
    public int FiremakingExperience { get; set; }
    public int CraftingRank { get; set; }
    public int CraftingLevel { get; set; }
    public int CraftingExperience { get; set; }
    public int SmithingRank { get; set; }
    public int SmithingLevel { get; set; }
    public int SmithingExperience { get; set; }
    public int MiningRank { get; set; }
    public int MiningLevel { get; set; }
    public int MiningExperience { get; set; }
    public int HerbloreRank { get; set; }
    public int HerbloreLevel { get; set; }
    public int HerbloreExperience { get; set; }
    public int AgilityRank { get; set; }
    public int AgilityLevel { get; set; }
    public int AgilityExperience { get; set; }
    public int ThievingRank { get; set; }
    public int ThievingLevel { get; set; }
    public int ThievingExperience { get; set; }
    public int SlayerRank { get; set; }
    public int SlayerLevel { get; set; }
    public int SlayerExperience { get; set; }
    public int FarmingRank { get; set; }
    public int FarmingLevel { get; set; }
    public int FarmingExperience { get; set; }
    public int RunecraftingRank { get; set; }
    public int RunecraftingLevel { get; set; }
    public int RunecraftingExperience { get; set; }
    public int HunterRank { get; set; }
    public int HunterLevel { get; set; }
    public int HunterExperience { get; set; }
    public int ConstructionRank { get; set; }
    public int ConstructionLevel { get; set; }
    public int ConstructionExperience { get; set; }
    public int SummoningRank { get; set; }
    public int SummoningLevel { get; set; }
    public int SummoningExperience { get; set; }
    public int DungeoneeringRank { get; set; }
    public int DungeoneeringLevel { get; set; }
    public int DungeoneeringExperience { get; set; }
    public int DivinationRank { get; set; }
    public int DivinationLevel { get; set; }
    public int DivinationExperience { get; set; }
    public int InventionRank { get; set; }
    public int InventionLevel { get; set; }
    public int InventionExperience { get; set; }

    public Player(string name, int rank, int totalLevel, int totalExperience, int attackRank, int attackLevel, int attackExperience, int defenceRank, int defenceLevel, int defenceExperience, int strengthRank, int strengthLevel, int strengthExperience, int constitutionRank, int constitutionLevel, int constitutionExperience, int rangedRank, int rangedLevel, int rangedExperience, int prayerRank, int prayerLevel, int prayerExperience, int magicRank, int magicLevel, int magicExperience, int cookingRank, int cookingLevel, int cookingExperience, int woodcuttingRank, int woodcuttingLevel, int woodcuttingExperience, int fletchingRank, int fletchingLevel, int fletchingExperience, int fishingRank, int fishingLevel, int fishingExperience, int firemakingRank, int firemakingLevel, int firemakingExperience, int craftingRank, int craftingLevel, int craftingExperience, int smithingRank, int smithingLevel, int smithingExperience, int miningRank, int miningLevel, int miningExperience, int herbloreRank, int herbloreLevel, int herbloreExperience, int agilityRank, int agilityLevel, int agilityExperience, int thievingRank, int thievingLevel, int thievingExperience, int slayerRank, int slayerLevel, int slayerExperience, int farmingRank, int farmingLevel, int farmingExperience, int runecraftingRank, int runecraftingLevel, int runecraftingExperience, int hunterRank, int hunterLevel, int hunterExperience, int constructionRank, int constructionLevel, int constructionExperience, int summoningRank, int summoningLevel, int summoningExperience, int dungeoneeringRank, int dungeoneeringLevel, int dungeoneeringExperience, int divinationRank, int divinationLevel, int divinationExperience, int inventionRank, int inventionLevel, int inventionExperience)
    {
        Name = name;
        Rank = rank;
        TotalLevel = totalLevel;
        TotalExperience = totalExperience;
        AttackRank = attackRank;
        AttackLevel = attackLevel;
        AttackExperience = attackExperience;
        DefenceRank = defenceRank;
        DefenceLevel = defenceLevel;
        DefenceExperience = defenceExperience;
        StrengthRank = strengthRank;
        StrengthLevel = strengthLevel;
        StrengthExperience = strengthExperience;
        ConstitutionRank = constitutionRank;
        ConstitutionLevel = constitutionLevel;
        ConstitutionExperience = constitutionExperience;
        RangedRank = rangedRank;
        RangedLevel = rangedLevel;
        RangedExperience = rangedExperience;
        PrayerRank = prayerRank;
        PrayerLevel = prayerLevel;
        PrayerExperience = prayerExperience;
        MagicRank = magicRank;
        MagicLevel = magicLevel;
        MagicExperience = magicExperience;
        CookingRank = cookingRank;
        CookingLevel = cookingLevel;
        CookingExperience = cookingExperience;
        WoodcuttingRank = woodcuttingRank;
        WoodcuttingLevel = woodcuttingLevel;
        WoodcuttingExperience = woodcuttingExperience;
        FletchingRank = fletchingRank;
        FletchingLevel = fletchingLevel;
        FletchingExperience = fletchingExperience;
        FishingRank = fishingRank;
        FishingLevel = fishingLevel;
        FishingExperience = fishingExperience;
        FiremakingRank = firemakingRank;
        FiremakingLevel = firemakingLevel;
        FiremakingExperience = firemakingExperience;
        CraftingRank = craftingRank;
        CraftingLevel = craftingLevel;
        CraftingExperience = craftingExperience;
        SmithingRank = smithingRank;
        SmithingLevel = smithingLevel;
        SmithingExperience = smithingExperience;
        MiningRank = miningRank;
        MiningLevel = miningLevel;
        MiningExperience = miningExperience;
        HerbloreRank = herbloreRank;
        HerbloreLevel = herbloreLevel;
        HerbloreExperience = herbloreExperience;
        AgilityRank = agilityRank;
        AgilityLevel = agilityLevel;
        AgilityExperience = agilityExperience;
        ThievingRank = thievingRank;
        ThievingLevel = thievingLevel;
        ThievingExperience = thievingExperience;
        SlayerRank = slayerRank;
        SlayerLevel = slayerLevel;
        SlayerExperience = slayerExperience;
        FarmingRank = farmingRank;
        FarmingLevel = farmingLevel;
        FarmingExperience = farmingExperience;
        RunecraftingRank = runecraftingRank;
        RunecraftingLevel = runecraftingLevel;
        RunecraftingExperience = runecraftingExperience;
        HunterRank = hunterRank;
        HunterLevel = hunterLevel;
        HunterExperience = hunterExperience;
        ConstructionRank = constructionRank;
        ConstructionLevel = constructionLevel;
        ConstructionExperience = constructionExperience;
        SummoningRank = summoningRank;
        SummoningLevel = summoningLevel;
        SummoningExperience = summoningExperience;
        DungeoneeringRank = dungeoneeringRank;
        DungeoneeringLevel = dungeoneeringLevel;
        DungeoneeringExperience = dungeoneeringExperience;
        DivinationRank = divinationRank;
        DivinationLevel = divinationLevel;
        DivinationExperience = divinationExperience;
        InventionRank = inventionRank;
        InventionLevel = inventionLevel;
        InventionExperience = inventionExperience;
    }

    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));
                    int StrengthRank = Convert.ToInt32(csv.GetField(10));
                    int StrengthLevel = Convert.ToInt32(csv.GetField(11));
                    int StrengthExperience = Convert.ToInt32(csv.GetField(12));
                    int ConstitutionRank = Convert.ToInt32(csv.GetField(13));
                    int ConstitutionLevel = Convert.ToInt32(csv.GetField(14));
                    int ConstitutionExperience = Convert.ToInt32(csv.GetField(15));
                    int RangedRank = Convert.ToInt32(csv.GetField(16));
                    int RangedLevel = Convert.ToInt32(csv.GetField(17));
                    int RangedExperience = Convert.ToInt32(csv.GetField(18));
                    int PrayerRank = Convert.ToInt32(csv.GetField(19));
                    int PrayerLevel = Convert.ToInt32(csv.GetField(20));
                    int PrayerExperience = Convert.ToInt32(csv.GetField(21));
                    int MagicRank = Convert.ToInt32(csv.GetField(22));
                    int MagicLevel = Convert.ToInt32(csv.GetField(23));
                    int MagicExperience = Convert.ToInt32(csv.GetField(24));
                    int CookingRank = Convert.ToInt32(csv.GetField(25));
                    int CookingLevel = Convert.ToInt32(csv.GetField(26));
                    int CookingExperience = Convert.ToInt32(csv.GetField(27));
                    int WoodcuttingRank = Convert.ToInt32(csv.GetField(28));
                    int WoodcuttingLevel = Convert.ToInt32(csv.GetField(29));
                    int WoodcuttingExperience = Convert.ToInt32(csv.GetField(30));
                    int FletchingRank = Convert.ToInt32(csv.GetField(31));
                    int FletchingLevel = Convert.ToInt32(csv.GetField(32));
                    int FletchingExperience = Convert.ToInt32(csv.GetField(33));
                    int FishingRank = Convert.ToInt32(csv.GetField(34));
                    int FishingLevel = Convert.ToInt32(csv.GetField(35));
                    int FishingExperience = Convert.ToInt32(csv.GetField(36));
                    int FiremakingRank = Convert.ToInt32(csv.GetField(37));
                    int FiremakingLevel = Convert.ToInt32(csv.GetField(38));
                    int FiremakingExperience = Convert.ToInt32(csv.GetField(39));
                    int CraftingRank = Convert.ToInt32(csv.GetField(40));
                    int CraftingLevel = Convert.ToInt32(csv.GetField(41));
                    int CraftingExperience = Convert.ToInt32(csv.GetField(42));
                    int SmithingRank = Convert.ToInt32(csv.GetField(43));
                    int SmithingLevel = Convert.ToInt32(csv.GetField(44));
                    int SmithingExperience = Convert.ToInt32(csv.GetField(45));
                    int MiningRank = Convert.ToInt32(csv.GetField(46));
                    int MiningLevel = Convert.ToInt32(csv.GetField(47));
                    int MiningExperience = Convert.ToInt32(csv.GetField(48));
                    int HerbloreRank = Convert.ToInt32(csv.GetField(49));
                    int HerbloreLevel = Convert.ToInt32(csv.GetField(50));
                    int HerbloreExperience = Convert.ToInt32(csv.GetField(51));
                    int AgilityRank = Convert.ToInt32(csv.GetField(52));
                    int AgilityLevel = Convert.ToInt32(csv.GetField(53));
                    int AgilityExperience = Convert.ToInt32(csv.GetField(54));
                    int ThievingRank = Convert.ToInt32(csv.GetField(55));
                    int ThievingLevel = Convert.ToInt32(csv.GetField(56));
                    int ThievingExperience = Convert.ToInt32(csv.GetField(57));
                    int SlayerRank = Convert.ToInt32(csv.GetField(58));
                    int SlayerLevel = Convert.ToInt32(csv.GetField(59));
                    int SlayerExperience = Convert.ToInt32(csv.GetField(60));
                    int FarmingRank = Convert.ToInt32(csv.GetField(61));
                    int FarmingLevel = Convert.ToInt32(csv.GetField(62));
                    int FarmingExperience = Convert.ToInt32(csv.GetField(63));
                    int RunecraftingRank = Convert.ToInt32(csv.GetField(64));
                    int RunecraftingLevel = Convert.ToInt32(csv.GetField(65));
                    int RunecraftingExperience = Convert.ToInt32(csv.GetField(66));
                    int HunterRank = Convert.ToInt32(csv.GetField(67));
                    int HunterLevel = Convert.ToInt32(csv.GetField(68));
                    int HunterExperience = Convert.ToInt32(csv.GetField(69));
                    int ConstructionRank = Convert.ToInt32(csv.GetField(70));
                    int ConstructionLevel = Convert.ToInt32(csv.GetField(71));
                    int ConstructionExperience = Convert.ToInt32(csv.GetField(72));
                    int SummoningRank = Convert.ToInt32(csv.GetField(73));
                    int SummoningLevel = Convert.ToInt32(csv.GetField(74));
                    int SummoningExperience = Convert.ToInt32(csv.GetField(75));
                    int DungeoneeringRank = Convert.ToInt32(csv.GetField(76));
                    int DungeoneeringLevel = Convert.ToInt32(csv.GetField(77));
                    int DungeoneeringExperience = Convert.ToInt32(csv.GetField(78));
                    int DivinationRank = Convert.ToInt32(csv.GetField(79));
                    int DivinationLevel = Convert.ToInt32(csv.GetField(80));
                    int DivinationExperience = Convert.ToInt32(csv.GetField(81));
                    int InventionRank = Convert.ToInt32(csv.GetField(82));
                    int InventionLevel = Convert.ToInt32(csv.GetField(83));
                    int InventionExperience = Convert.ToInt32(csv.GetField(84));

                    return new Player(name, Rank, TotalLevel, TotalExperience, AttackRank, AttackLevel, AttackExperience, DefenceRank, DefenceLevel, DefenceExperience, StrengthRank, StrengthLevel, StrengthExperience, ConstitutionRank, ConstitutionLevel, ConstitutionExperience, RangedRank, RangedLevel, RangedExperience, PrayerRank, PrayerLevel, PrayerExperience, MagicRank, MagicLevel, MagicExperience, CookingRank, CookingLevel, CookingExperience, WoodcuttingRank, WoodcuttingLevel, WoodcuttingExperience, FletchingRank, FletchingLevel, FletchingExperience, FishingRank, FishingLevel, FishingExperience, FiremakingRank, FiremakingLevel, FiremakingExperience, CraftingRank, CraftingLevel, CraftingExperience, SmithingRank, SmithingLevel, SmithingExperience, MiningRank, MiningLevel, MiningExperience, HerbloreRank, HerbloreLevel, HerbloreExperience, AgilityRank, AgilityLevel, AgilityExperience, ThievingRank, ThievingLevel, ThievingExperience, SlayerRank, SlayerLevel, SlayerExperience, FarmingRank, FarmingLevel, FarmingExperience, RunecraftingRank, RunecraftingLevel, RunecraftingExperience, HunterRank, HunterLevel, HunterExperience, ConstructionRank, ConstructionLevel, ConstitutionExperience, SummoningRank, SummoningLevel, SummoningExperience, DivinationRank, DivinationLevel, DivinationExperience, InventionRank, InventionLevel, InventionExperience);
                }
            }
        }
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Holy smoke! ;-] \$\endgroup\$ – t3chb0t Jan 21 '17 at 16:08
  • 1
    \$\begingroup\$ Off topic: runescape? \$\endgroup\$ – Eric Lagergren Jan 21 '17 at 21:30
  • \$\begingroup\$ @EricLagergren yeah, it's to use the provided API. \$\endgroup\$ – iub3rskillz Jan 21 '17 at 21:52
  • 1
    \$\begingroup\$ @iub3rskillz cool! Haven't played the game in ages, but the names sounded awfully familiar :) \$\endgroup\$ – Eric Lagergren Jan 21 '17 at 22:16
16
\$\begingroup\$

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<T>(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<int>(1);
int TotalLevel =csv.GetField<int>(2);
int TotalExperience = csv.GetField<int>(3);
int AttackRank = csv.GetField<int>(4);
int AttackLevel = csv.GetField<int>(5);
int AttackExperience = csv.GetField<int>(6);
int DefenceRank = csv.GetField<int>(7);
int DefenceLevel = csv.GetField<int>(8);
int DefenceExperience = csv.GetField<int>(9);

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

Total = new PlayerStat(csv.GetField<int>(1), csv.GetField<int>(2), csv.GetField<int>(3));
Attack = new PlayerStat(csv.GetField<int>(4), csv.GetField<int>(5), csv.GetField<int>(6));
Defence = new PlayerStat(csv.GetField<int>(7), csv.GetField<int>(8), csv.GetField<int>(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 < properties.Length; i++)
{
    var thisStat = new PlayerStat(csv.GetField<int>(i * 3 + 1), csv.GetField<int>(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 CsvAttribute(int startPosition)
    {
        StartPosition = startPosition;
    }
}

Then, on each property in the class:

[Csv(1)]
public PlayerStat Total { get; set; }
[Csv(2)]
public PlayerStat Attack { get; set; }
[Csv(3)]
public PlayerStat Defence { get; set; }

Then this:

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

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

Becomes this:

PropertyInfo[] properties = typeof(Player).GetProperties()
    .Where(prop => Attribute.IsDefined(prop, typeof(CsvAttribute))).ToArray();

for (int i = 0; i < properties.Length; i++)
{
    var start = (properties[i].GetCustomAttributes(true).First(x => x is CsvAttribute) as CsvAttribute).StartPosition * 3;
    var thisStat = new PlayerStat(csv.GetField<int>(start + 0), csv.GetField<int>(start + 1), csv.GetField<int>(start + 2));
    typeof(Player).InvokeMember(properties[i].Name, BindingFlags.Public | BindingFlags.Instance | BindingFlags.SetProperty, Type.DefaultBinder, this, new object[] { thisStat });
}

Now order doesn't matter. Just define Csv(int) with the position that the stat is in. You could rewrite it so that StartPosition is the * 3 position, either one is up to you.

This also means you can add, remove and change them easily, really easily.


As recommended in comments, an enum for each property would be helpful as well:

public enum PlayerStatField
{
    Rank = 1,
    Level = 2,
    Experience = 3,
}

Then we can use it in our method:

PropertyInfo[] properties = typeof(Player).GetProperties()
    .Where(prop => Attribute.IsDefined(prop, typeof(CsvAttribute))).ToArray();

for (int i = 0; i < properties.Length; i++)
{
    var start = (properties[i].GetCustomAttributes(true).First(x => x is CsvAttribute) as CsvAttribute).StartPosition * 3;
    var thisStat = new PlayerStat(
        csv.GetField<int>(start + (int)PlayerStatField.Rank),
        csv.GetField<int>(start + (int)PlayerStatField.Level),
        csv.GetField<int>(start + (int)PlayerStatField.Experience));
    typeof(Player).InvokeMember(properties[i].Name,
                                BindingFlags.Public | BindingFlags.Instance | BindingFlags.SetProperty,
                                Type.DefaultBinder,
                                this,
                                new object[] { thisStat });
}

All this work brings your original class down to:

public class Player
{
    public string Name { get; private set; }
    [Csv(0)] public PlayerStat Total { get; private set; }
    [Csv(1)] public PlayerStat Attack { get; private set; }
    [Csv(2)] public PlayerStat Defence { get; private set; }
    [Csv(3)] public PlayerStat Strength { get; private set; }
    [Csv(4)] public PlayerStat Constitution { get; private set; }
    [Csv(5)] public PlayerStat Ranged { get; private set; }
    [Csv(6)] public PlayerStat Prayer { get; private set; }
    [Csv(7)] public PlayerStat Magic { get; private set; }
    [Csv(8)] public PlayerStat Cooking { get; private set; }
    [Csv(9)] public PlayerStat Woodcutting { get; private set; }
    [Csv(10)] public PlayerStat Fletching { get; private set; }
    [Csv(11)] public PlayerStat Fishing { get; private set; }
    [Csv(12)] public PlayerStat Firemaking { get; private set; }
    [Csv(13)] public PlayerStat Crafting { get; private set; }
    [Csv(14)] public PlayerStat Smithing { get; private set; }
    [Csv(15)] public PlayerStat Mining { get; private set; }
    [Csv(16)] public PlayerStat Herblore { get; private set; }
    [Csv(17)] public PlayerStat Agility { get; private set; }
    [Csv(18)] public PlayerStat Thieving { get; private set; }
    [Csv(19)] public PlayerStat Slayer { get; private set; }
    [Csv(20)] public PlayerStat Farming { get; private set; }
    [Csv(21)] public PlayerStat Runecrafting { get; private set; }
    [Csv(22)] public PlayerStat Hunter { get; private set; }
    [Csv(23)] public PlayerStat Construction { get; private set; }
    [Csv(24)] public PlayerStat Summoning { get; private set; }
    [Csv(25)] public PlayerStat Dungeoneering { get; private set; }
    [Csv(26)] public PlayerStat Divination { get; private set; }
    [Csv(27)] public PlayerStat Invention { get; private set; }

    public void GetHiscore()
    {
        var _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 (var _response = (HttpWebResponse)_request.GetResponse())
        using (var _reader = new StreamReader(_response.GetResponseStream(), Encoding.UTF8, false))
        using (var csv = new CsvReader(_reader))
        {
            var properties = typeof(Player).GetProperties()
                .Where(prop => Attribute.IsDefined(prop, typeof(CsvAttribute))).ToArray();

            for (int i = 0; i < properties.Length; i++)
            {
                var start = (properties[i].GetCustomAttributes(true).First(x => x is CsvAttribute) as CsvAttribute).StartPosition * 3;
                var thisStat = new PlayerStat(
                    csv.GetField<int>(start + (int)PlayerStatField.Rank),
                    csv.GetField<int>(start + (int)PlayerStatField.Level),
                    csv.GetField<int>(start + (int)PlayerStatField.Experience));
                typeof(Player).InvokeMember(
                    properties[i].Name,
                    BindingFlags.Public | BindingFlags.Instance | BindingFlags.SetProperty,
                    Type.DefaultBinder,
                    this,
                    new object[] { thisStat });
            }
        }
    }
}

That's a nice change.

| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ There are so many properties that I didn't even realize there was a method to get the CSV from a server. \$\endgroup\$ – t3chb0t Jan 21 '17 at 16:25
  • 2
    \$\begingroup\$ I'd also suggest to use an enum with values holding column number to get rid of magic numbers. Enums can also be used to hold information about datatypes public enum PlayerFields { Name = 1, Rank = 2, TotalLevel = 3 ... } \$\endgroup\$ – bushed Jan 21 '17 at 18:30
  • \$\begingroup\$ This is the sollution for me, I only used the first half because the latter was not really needed in my application. \$\endgroup\$ – iub3rskillz Jan 21 '17 at 20:20
  • \$\begingroup\$ @bushed That's fair. \$\endgroup\$ – Der Kommissar Jan 21 '17 at 20:34
  • 1
    \$\begingroup\$ @iub3rskillz That's fair, I try to write answers that are more broad and useable by more people. I'm glad it was helpful. :) \$\endgroup\$ – Der Kommissar Jan 21 '17 at 20:40
3
\$\begingroup\$

I find you should split it either into multiple CSVs - one per player with his properties or a larger one with all player properties.

The Rank, Level and Experience repeat for every properety set. Encapsulate them in a new class:

class PlayerProperty
{
    public string PlayerName { get; set; }
    public string PropertyName { get; set; }
    public int Rank { get; private set; }
    public int Level { get; private set; }
    public int Experience { get; private set; }
}

Now the new CSV will only have 5 column and you can easily add new ones if necessary. The Player class will hold just the basic properties... oh, wait, the Player class seem to be no longer necessary because if we refactor it the only property that is left is the Name property.


With your current solution you don't need the helper variables and the constructor at all. All properties are read/write so you could just use the object initializer to create a Player:

using (CsvReader csv = new CsvReader(_reader))
{
    return Player
    {
        Rank = Convert.ToInt32(csv.GetField(1)),
        ..
    }
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.