Guazz
Guazz

Reputation: 21

C# Setting Properties using Index

I have a business class that contains many properties for various stock-exchange price types. This is a sample of the class:

public class Prices
{
    public decimal Today {get; set;}
    public decimal OneDay {get; set;}
    public decimal SixDay {get; set;}
    public decimal TenDay {get; set;}
    public decimal TwelveDay {get; set;}
    public decimal OneDayAdjusted {get; set;}
    public decimal SixDayAdjusted {get; set;}
    public decimal TenDayAdjusted {get; set;}
    public decimal OneHundredDayAdjusted {get; set;}
}

I have a legacy system that supplies the prices using string ids to identify the price type.

E.g.

Today = "0D"  
OneDay = "1D"  
SixDay = "6D"  
//..., etc.   

Firstly, I load all the values to an IDictionary() collection so we have:

[KEY] VALUE
[0D] => 1.23456
[1D] => 1.23456
[6D] => 1.23456
...., etc.

Secondly, I set the properties of the Prices class using a method that takes the above collection as a parameter like so:

SetPricesValues(IDictionary<string, decimal> pricesDictionary)  
{  
    // TODAY'S PRICE  
    string TODAY = "D0";  
    if (true == pricesDictionary.ContainsKey(TODAY))  
    {  
        this.Today = pricesDictionary[TODAY];  
    }  
    // OneDay PRICE  
    string ONE_DAY = "D1";  
    if (true == pricesDictionary.ContainsKey(ONE_DAY))  
    {  
         this.OneDay = pricesDictionary[ONE_DAY];  
    }  
//..., ..., etc., for each other property   
}  

Is there a more elegant technique to set a large amount of properties? Thanks, j

Upvotes: 2

Views: 3561

Answers (5)

Abel
Abel

Reputation: 57217

The way I see it, you have a few options, depending on your skills, the way you are allowed to change the current POCO's or other classes:

  • If you must use a dictionary, create a similar dictionary which maps the "0D" etc to the OneDay names. Loop through the dictionary and assign using simple reflection.
  • If you can change the way the data is read, have the dictionary read with OneDay etc, instead of the "0D", which is only applicable to the external application.
  • Create an attribute, LegacyKeyAttribute, augment your POCO gettors/settors with this attribute. Now it becomes trivial: loop through the properties of the POCO to find the correct property for your current legacy key.

The last option requires a bit more understanding of C# than many average programmers know: writing and using attributes and reflection. However, in the end it's the cleanest and easiest solution (I'll try to come up with an example).


UPDATE: here's a little example. Meanwhile, many improvement suggestions have been posted, but none still uses attributes, while your case seems ideal. Why? It poses the least burden on existing code, I believe, and it makes reading and understanding your code even easier.

Usage:

// any price:
Prices prices = new Prices();
prices.SetPriceByLegacyName("0D", 1.2345M);

// or, your loop becomes a bit easier:
SetPricesValues(IDictionary<string, decimal> pricesDictionary)  
{  
    foreach(string key in pricesDictionary.Keys)
    {
        // assuming "this" is of type Prices (you didn't specify)
        this.SetPriceByLegacyName(key, pricesDictionary[key]);
    }    
}  

The implementation:

// the simplest attribute class is enough for you:
[AttributeUsage(AttributeTargets.Property)]
public class LegacyNameAttribute : Attribute
{
    public string Name { get; set; }
    public LegacyNameAttribute(string name)
    {
        this.Name = name;
    }
}

// your Prices POCO class becomes easier to read
public class Prices
{
    [LegacyName("0D")]    public decimal Today { get; set; }
    [LegacyName("1D")]    public decimal OneDay { get; set; }
    [LegacyName("6D")]    public decimal SixDay { get; set; }
    [LegacyName("10D")]   public decimal TenDay { get; set; }
    [LegacyName("12D")]   public decimal TwelveDay { get; set; }
    [LegacyName("1DA")]   public decimal OneDayAdjusted { get; set; }
    [LegacyName("6DA")]   public decimal SixDayAdjusted { get; set; }
    [LegacyName("10DA")]  public decimal TenDayAdjusted { get; set; }
    [LegacyName("100DA")] public decimal OneHundredDayAdjusted { get; set; }
}

// an extension method to ease the implementation:
public static class PricesExtensions
{
    public static void SetPriceByLegacyName(this Prices price, string name, decimal value)
    {
        if (price == null)
            throw new ArgumentException("Price cannot be null");

        foreach (PropertyInfo prop in price.GetType().GetProperties())
        {
            LegacyNameAttribute legNameAttribute = (LegacyNameAttribute)
                Attribute.GetCustomAttribute(prop, typeof(LegacyNameAttribute));

            // set the property if the attribute matches
            if (legNameAttribute != null && legNameAttribute.Name == name)
            {
                prop.SetValue(price, value, null);
                break;   // nothing more to do
            }
        }
    }
}

That's all there is to it. Even with all the added lines, it may well be that your total line count becomes less. But more importantly, it becomes easier to maintain and use.

Upvotes: 0

Aaronaught
Aaronaught

Reputation: 122684

Instead of using a string-to-decimal mapping and checking the dictionary repeatedly, use a delegate mapping/extension method:

public static class PriceConverter
{
    private static readonly Dictionary<string, Action<Prices, decimal>> setters =
        CreateSetterDictionary();

    public static void SetPrice(this Prices p, string id, decimal newPrice)
    {
        Action<Prices, decimal> setter;
        if (setters.TryGetValue(id, out setter))
            setter(p, newPrice);
    }

    private static Dictionary<string, Action<Prices, decimal>>
        CreateSetterDictionary()
    {
        var dic = new Dictionary<string, Action<Prices, decimal>>();
        dic.Add("0D", (p, d) => p.Today = d);
        dic.Add("1D", (p, d) => p.OneDay = d);
        // etc.
        return dic;
    }
}

Then you can write prices.SetPrice("0D", 1.23456).

If you like, add a throw statement at the end of the SetPrice method to handle cases where the id doesn't match anything.

Upvotes: 2

Sunny
Sunny

Reputation: 6346

Just an idea:

interface IPrices_As_String{
 string OD { get; set; }
 // other properties here...
}

interface IPrices{
 decimal Today{get; set;}
}

class Prices : IPrices, IPrices_As_String{
 public decimal Today { get; set; }
 public string IPrices_As_String.OD {
  get { return this.Today.ToString(); }
  set { 
    if(!String.IsNullOrEmpty(value)){
       this.Today = decimal.Parse(value);
    }
  }
 }
}

Then when I am setting the values from the legacy system, I will use the Prices class on the interface as IPrices_As_String like:

IPrices_As_String obj = new Prices();
// set values from the legacy system

IPrices obj2 = obj as IPrices; // will give me the correct object..

.

HTH.

Upvotes: 0

Danny Varod
Danny Varod

Reputation: 18118

Define a dictionary of properties in the constructor e.g.

private Dictionary<int, PropertyInfo> propertyDictionary = new ...

MyClass()
{
    this.propertyDictionary.Add(0, this.GetType().GetProperty("FirstProperty");
    ...
}

then access using an indexed property

decimal this[int index]
{
    get
    {
        PropertyInfo property;
        if (this.propertyDictionary.TryGetValue(index, out property))
        {
            // Not sure I remember the arguments right here:
            property.SetValue(this, new object[] { value });
        }
    set
    {
        // Similar code
    }
}

You could later on improve this code by automatically parsing the properties in the constructor using reflection, adding all properties with an attribute that tells you what the id is. (Instead of adding them manually in the constructor).

Upvotes: 0

Ray
Ray

Reputation: 21905

I would put the string variables into constants, rather than declare them every time you run the method:

private const string ONE_DAY = "D1";

If you expect the collection parameter to contain all or most of the possible values, then your code is probably cool. If you expect that the dictionary will have a small subset of the possible values, it might be more efficient to use a foreach loop and a switch statement to set values, rather then do a lookup for every possible value every time. It just depends on how many values you need to deal with and how many you get in each method call.

Upvotes: 0

Related Questions