User editable properties with varying number of arguments

Question

Recently I used this pattern to create a list of properties, each property having a varying number of arguments.

The properties list can be made at compile time, it does not need to change at runtime.

I tried to code this in a way such that it was easy for me to edit to add in additional properties (i.e., easy to add in a "Video Card" with arguments "Brand", and "Cost").

This would be part of an application, e.g., this could be implemented by having a "property editor" dialog which showed the property name, and 3 text boxes with labels showing the names of the properties they are editing.

• The constructor for clsProperty has a lot of exceptions it can throw, it's probably not necessary since this can only be changed at compile time, but I put them in for completeness and to demonstrate what could go wrong if there was a typo in the code that made the properties list; let me know if you think there is a better way to do error handling in this case. It would be nice to have at least some sort of warning if the parameters aren't right but I'm not sure if exceptions are the right thing to use here.

• I think I saw that a list of Object is usually frowned upon, let me know if this is a valid time to use that, or if there is an alternative in this case.

• Any general suggestions to improve readability would be helpful too.

For the sake of this example assume that everything in clsProperty is for display purposes only, no code will be doing something like if(Property.Name == ...)

using System;
using System.Collections.Generic;
namespace csParseTest
{
 internal class clsProperty
 {
 // is it worth making these fields into properties of clsProperty?
 internal readonly string Name; 
 internal readonly List<string> ArgNames = new List<string>();
 internal readonly List<Type> ArgTypes = new List<Type>();
 internal readonly List<Object> ArgDefaults = new List<Object>();
 private List<Object> ArgValues = new List<Object>();
 internal clsProperty(string iName, List<string> iArgNames, List<Type> iArgTypes, List<Object> iArgDefaults)
 {
 if (string.IsNullOrWhiteSpace(iName))
 {
 throw new ArgumentException("Property must have a valid name.");
 }
 this.Name = iName;
 bool tParameterLengthsOK = (
 (iArgNames.Count == iArgTypes.Count) && 
 (iArgTypes.Count == iArgDefaults.Count)
 );
 if (!tParameterLengthsOK)
 {
 throw new ArgumentException("Each argument must have a name, type, and default value.");
 }
 for (int tArgumentIndex = 0; (tArgumentIndex < iArgNames.Count); tArgumentIndex++)
 {
 string tArgName = iArgNames[tArgumentIndex];
 Type tArgType = iArgTypes[tArgumentIndex];
 object tDefaultValue = iArgDefaults[tArgumentIndex];
 if(tDefaultValue == null)
 {
 throw new ArgumentException("Argument " + tArgumentIndex + ": no default value.");
 }
 if (String.IsNullOrWhiteSpace(tArgName))
 {
 throw new ArgumentException("Argument " + tArgumentIndex + ": name must not be blank.");
 }
 if (!IsValidArgType(tArgType))
 {
 throw new ArgumentException("Argument " + tArgumentIndex + ": type " + tArgType.ToString() + " is not supported.");
 }
 this.ArgNames.Add(tArgName);
 this.ArgTypes.Add(tArgType);
 this.ArgDefaults.Add(tDefaultValue);
 this.ArgValues.Add(tDefaultValue);
 }
 }
 internal void SetArgument(int iArgIndex, string iValueString)
 { 
 if ((iArgIndex < 0) || (iArgIndex > (ArgValues.Count - 1)))
 {
 throw new IndexOutOfRangeException("Argument index " + iArgIndex + " is invalid, arguments list has " + ArgValues.Count + " items.");
 }
 Object tValue = null;
 Type tArgType = this.ArgTypes[iArgIndex];
 if (TryParseArgValue(iValueString, tArgType, out tValue))
 {
 this.ArgValues[iArgIndex] = tValue;
 }
 else
 {
 throw new ArgumentException("Could not parse string " + iValueString + " as argument type " + tArgType.ToString());
 }
 }
 // for the sake of this example assume that properties can only be one of these types.
 internal static bool IsValidArgType(Type iType)
 {
 return
 (
 (iType == typeof(System.String)) ||
 (iType == typeof(System.Int32)) ||
 (iType == typeof(System.Int64)) ||
 (iType == typeof(System.Single)) ||
 (iType == typeof(System.Double))
 ); 
 }
 private static bool TryParseArgValue(string iValueString, Type iParseAsType, out object qValue)
 {
 qValue = null;
 if (iParseAsType == typeof(System.String))
 {
 qValue = iValueString;
 return true;
 }
 else if (iParseAsType == typeof(System.Int32))
 {
 int tParsedInt;
 if (int.TryParse(iValueString, out tParsedInt))
 {
 qValue = tParsedInt;
 return true;
 }
 else
 {
 return false;
 }
 }
 else if (iParseAsType == typeof(System.Int64))
 {
 long tParsedLong;
 if (long.TryParse(iValueString, out tParsedLong))
 {
 qValue = tParsedLong;
 return true;
 }
 else
 {
 return false;
 }
 }
 else if (iParseAsType == typeof(System.Single))
 {
 float tParsedFloat;
 if (float.TryParse(iValueString, out tParsedFloat))
 {
 qValue = tParsedFloat;
 return true;
 }
 else
 {
 return false;
 }
 }
 else if (iParseAsType == typeof(System.Double))
 {
 double tParsedDouble;
 if (double.TryParse(iValueString, out tParsedDouble))
 {
 qValue = tParsedDouble;
 return true;
 }
 else
 {
 return false;
 }
 }
 else
 {
 // I considered throwing an exception here
 return false;
 }
 }
 };
 class Program
 {
 // for the sake of this example assume that this properties list does not need to be changed at runtime.
 private static readonly List<clsProperty> AllProperties = new List<clsProperty>
 {
 new clsProperty("Optical Drive", 
 new List<string> {"Brand", "Speed", "Format"}, 
 new List<Type> {typeof(System.String), typeof(System.Single), typeof(System.String)}, 
 new List<Object> {"Asus", 220.3, "BD-ROM"}),
 // or with named parameters...
 new clsProperty(iName:"Hard Drive", 
 iArgNames:new List<string> {"Type", "Price"}, 
 iArgTypes: new List<Type> {typeof(System.String), typeof(System.Double)}, 
 iArgDefaults: new List<Object> {"Solid State Drive", 123.4})
 };
 // test code
 static void Main(string[] args)
 {
 // e.g., user edits values using text boxes (or a data grid maybe),
 // then hits OK or apply
 AllProperties[0].SetArgument(0, "Lite-On");
 AllProperties[0].SetArgument(1, "100.9");
 AllProperties[0].SetArgument(2, "DVD-ROM");
 //Properties[0].SetArgument(1, "ShouldFail");
 return;
 }
 }
}

EDIT: This would be using Winforms for the GUI

• Sophie Swett: Rather than having three separate lists (one for names, one for types, and one for values), it would probably be better to have just one list, where each item in the list contains a name, a type, and a value. Also consider using a dictionary instead of a list, because looking up strings in a dictionary is both faster and easier than looking them up in a list.
• jrh: Just wondering, were you referring to the input parameters to the constructor, the class's internal fields, or both? Also, you're right that a dictionary could help access properties by name, but for right now the only thing needed is access by index.
• RubberDuck: To expand on what @TannerSwett said, I'd probably use a Dictionary<string, Tuple<Type, Object>>. I'm just curious though. What's the purpose of this code? What problem are you trying to solve? If we knew the reason you felt the need to write this, we may be able to provide a completely different alternative. (Possibly something using dynamic, but I don't know...)
• Sophie Swett: @jrh I was referring to both. Though, as a bit of a counterpoint, the constructor might be easier to use if it takes a list (or even an IEnumerable) instead of a dictionary.
• RubberDuck: And why wouldn't you just write the appropriate classes @jrh? At least you'd have some static typing to help you avoid bugs. The solution you have here is going to be much more susceptible to runtime errors.

Answer 1

Is it about web or windows UI? WPF or WinForms: you could just use PropertyGrid - it is the same component you have in Visual Studio to edit properties. To define properties at runtime if happens to be needed - see TypeDescriptor.

• jrh: A PropertyGrid seems like a good fit for this case. RubberDuck mentioned in chat that I could use metadata to make properties with user-friendly, translatable name attributes. When I wrote this, I wasn't aware of how much you could do with metadata, or that there were controls that could access it, so this is pretty cool! I'm still not completely sure where metadata is stored, or the best way to make it work with a resx file, but this is probably the direction I'm going to go in.
• Dmitry Nogin: Yep, you can find a lot about this here, here or Chapter 11 Properties Window in this book. You can do a lot there.

Answer 2

Naming

Your names are a little confusing. Arguments are what you're passing into methods or applications. What you're calling arguments here are normally called properties or attributes.

Class names start with a capital letter by convention, so that would make ClsProperty.

I wouldn't use abbreviations here - ClassProperty is just a few characters longer and reads a lot nicer. I'm not sure if it's a very descriptive name though - but without knowing more about the context in which it will be used it's hard to suggest a better name.

I assume the i, t and q prefixes stand for 'argument', 'local' and 'out' respectively? Personally I don't use such prefixes - to me they add little value, nothing that a descriptive name and a mouseover in a decent IDE doesn't tell me. Either way, when you change a local variable to an argument you'll have to update its prefix too, and that's easily forgotten, which makes these kind of prefixes less reliable over time.

SetArgument doesn't set an argument (attribute), it sets the value of that argument, so SetArgumentValue would be a better name.

Code structure

Don't use separate lists for things that belong together - create an Attribute class that contains a name, type, default value and current value. It'll make your code easier to understand and to maintain. For example, you no longer have to deal with lists of different length, and horizontally aligning names, types and default values is no longer necessary either.

A ClassProperty probably shouldn't have multiple attributes with the same name, so storing attributes in a dictionary (using their name as key) and checking for duplicate names would be a good idea.

ClassProperty is already marked as internal, so you don't need to mark its properties as internal again. Doing so signals that they're not meant to be used by external code. You may actually want to make those lists (or that dictionary) private, and expose those attributes via a read-only array or IEnumerable property instead, so outside code can access and modify all attributes but can't add, remove or replace any.

Initialization

You can use the params keyword if you want to call a method (or constructor) with a variable number of arguments (it's syntactic sugar for creating an array at the call-site):

// Constructor signature:
public ClassProperty(string name, params Attribute[] attributes) { ... }
// Call with as many or as few Attribute arguments as you want:
new ClassProperty("Optical Drive",
 new Attribute("Brand", typeof(string), "Asus"),
 new Attribute("Speed", typeof(float), 220.3),
 new Attribute("Format", typeof(string), "BD-ROM"));

Alternately, you can let ClassProperty implement IEnumerable and give it an Add method so you can use initializer syntax with it:

new ClassProperty("Optical Drive") {
 new Attribute("Brand", typeof(string), "Asus"),
 new Attribute("Speed", typeof(float), 220.3),
};

I'd prefer the params approach however, as it doesn't clutter the interface of your ClassProperty class. You also mentioned that new attributes won't be added at run-time, so it may not be desirable to have an Add method anyway.

Type safety

Your initialization code does not check if a given default value is valid for the specified attribute type. With a little use of generics, you can enforce correct typing at compile-time:

// Static method in Attribute:
public static Attribute Create<T>(string name, T value)
{
 return new Attribute(name, typeof(T), value);
}
new ClassProperty("Optical Drive",
 Attribute.Create<string>("Brand", "Asus"), // <- fine
 Attribute.Create<float>("Speed", "BD-ROM")); // <- compile error!

Other notes

Your use of type-names is somewhat inconsistent. Since System.String and string refer to the same type, I would just use string everywhere - it's less verbose and everyone's familiar with it anyway. The same goes for the other System.* types.

TryParseArgValue contains a fair bit of code. You could simplify this by using Convert.To* methods instead of *.TryParse methods. If an exception is thrown, simply return false. Try* methods don't throw by convention, so you were right by deciding to return false for unsupported types.

Documentation! Attributes can only use a select few types, and null is not a valid default value. That's the kind of thing you want to put in the summary docstring of the relevant class or method.

• jrh: Thanks for looking through it. I think an attribute class is a good idea; would it be better to make a new Attribute every time its Value is changed (in this case would a mutable class be good practice?), or should I use Attribute.SetValue() or something similar? Another question, if I used generics, how would I access the type of the attribute when doing Convert.To*?
• jrh: Also regarding "Properties" / "Arguments" naming, I'm using the naming from the program I'm exporting this "configuration file" to. For the prefixes: part of it is that I go back and forth between VB.NET and C# and case isn't enough to differentiate between instances and definitions. The tiq prefix thing I admit might only help me.
• Pieter Witvoet: I would only make the value of that attribute class mutable. If it's immutable you'll need to replace attributes, which allows calling code to set a new name or type. Preventing that then requires additional checks... which is making things more complicated for no real gain. As for generics, I don't think there's much value in making the attribute class itself generic. A generic helper method that creates non-generic instances (as shown above) is sufficient in this case.