Question

I have some code that is supposed to assign some values to its properties according to a given order.

Here is some sample code that will do the job:

public class AnimalCount
{
    public int Dogs;
    public int Cats;
    public int Fish;
    public int Birds;

    public void RankValues(string first, string second, string third, string fourth)
    {
        string property = "";
        int value = -1;
        for (int i = 0; i < 4; i++)
        {
            switch (i)
            {
                case 0: property = first;  value = 10; break;
                case 1: property = second; value = 12; break;
                case 2: property = third;  value = 19; break;
                case 3: property = fourth; value = 20; break;
            }
            switch (property)
            {
                case "dogs":  Dogs  = value; break;
                case "cats":  Cats  = value; break;
                case "fish":  Fish  = value; break;
                case "birds": Birds = value; break;
            }
        }
    }
}

There are some problems with this code, however.

  1. The main problem is how to pass the parameters. With this method, since they are passed as strings, we lose type safety. We could thus have duplicates or mismatched strings. We could use enums, but then we still have the risk of duplicates, and we would have to do some code duplication to make it work.
  2. The switches are ugly. It feels like code duplication somehow.

Is there any better solution than just filling the code with exception handling? Seems awfully ugly to me.

If you must know, I'm trying to write a function that takes a requested order of ability scores in Dungeons and Dragons and rolls for them in your selected order.

Was it helpful?

Solution

I would do this:

public class AnimalCount
{
    public int Dogs;
    public int Cats;
    public int Fish;
    public int Birds;

    private Dictionary<string, Action<int>> rankers
        = new Dictionary<string, Action<int>>()
    {
        { "dogs", v => Dogs = v },
        { "cats", v => Cats = v },
        { "fish", v => Fish = v },
        { "birds", v => Birds = v },
    };

    private Action<string, int> setRank = (t, v) =>
    {
        if (rankers.ContainsKey(t))
        {
            rankers[t](v);
        }
    };

    public RankValues(string first, string second, string third, string fourth)
    {
        setRank(first, 10);
        setRank(second, 12);
        setRank(third, 19);
        setRank(fourth, 20);
    }
}

OTHER TIPS

Not entirely sure if I'm following you, but can you not simply take in an ordered collection of 4 parameters?

public void doWhatever(String[] orderedParams) {
    this.animals = orderedParams;
    // ... 
    this.doTheThing(animals[0], 10);
    this.doTheThing(animals[1], 12);
    // etc
}

A dictionary would be a nice container for your results, as you are essentially wanting a key/value pair. If you feed two collections into your rank function

public Dictionary<string, int> Rank(string[] orderedKeys, int[] orderedValues)
{
    Dictionary<string, int> rankedDictionary = new Dictionary<string, int>();
    for (int i = 0; i < orderedKeys.Length; i++)
    {
        rankedDictionary.Add(orderedKeys[i], orderedValues[i]);
    }
    return rankedDictionary;
}

public void CallRank()
{
    string[] orderedKeys = new[] { "dogs", "cats", "fish", "birds" };
    int[] orderedValues = new[] { 10, 12, 19, 20 };

    Dictionary<string,int> rankedResults =  Rank(orderedKeys, orderedValues);

    int catsValue = rankedResults["cats"];
}

The reason I asked if you were using C# is because if you are worried about strongly typed variables, instead of using strings of "cat" and "dog" etc. You can use an Enum in c#.

http://msdn.microsoft.com/en-us/library/sbbt4032.aspx

public enum Animals
{
    Dog
    Cat
    ....
}

so you dictionary would be of type

Dictionary<Animals, int>

and you would access it as so

int dogValue = rankedDictionary[Animals.Dog];

Taking ideas from the other answers, I believe the best way to implement this would be the following:

using System.Collections.Generic;
public class AnimalCount
{
    public int Dogs { get { return animals["dogs"]; } }
    public int Cats { get { return animals["cats"]; } }
    public int Fish { get { return animals["fish"]; } }
    public int Birds { get { return animals["birds"]; } }

    private Dictionary<string, int> animals = new Dictionary<string, int>();

    public void RankValues(string first, string second, string third, string fourth)
    {
        animals[first] = 10;
        animals[second] = 12;
        animals[third] = 19;
        animals[fourth] = 20;
    }
}

and with an enum for type safety:

using System.Collections.Generic;

public enum Animals
{
    Dogs, Cats, Fish, Birds
}

public class AnimalCount
{
    public int Dogs { get { return animals[Animals.Dogs]; } }
    public int Cats { get { return animals[Animals.Cats]; } }
    public int Fish { get { return animals[Animals.Fish]; } }
    public int Birds { get { return animals[Animals.Birds]; } }

    private Dictionary<Animals, int> animals = new Dictionary<Animals, int>();

    public void RankValues(Animals first, Animals second, Animals third, Animals fourth)
    {
        animals[first] = 10;
        animals[second] = 12;
        animals[third] = 19;
        animals[fourth] = 20;
    }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top