User editable properties with varying number of arguments

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 ArgNames = new List();
internal readonly List ArgTypes = new List();
internal readonly List ArgDefaults = new List();

private List ArgValues = ne

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(string name, T value)
{
    return new Attribute(name, typeof(T), value);
}

new ClassProperty("Optical Drive",
    Attribute.Create("Brand", "Asus"),    // ("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.

StackExchange Code Review Q#129115, answer score: 4