views:

356

answers:

4

Hello.

I am creating a network chat client in C# as a side project. In addition to simple text messages, I also have slash-prefixed commands that can be entered into the input TextBox. I used a modular approach by creating an enum that contains all the various commands, and then decorating those commands with attributes.

The attributes specify what slash-prefixed command can be entered to trigger the command, as well as any aliases to the primary command identifier and the command's usage.

Example:

public enum CommandType : byte
{
    [PrimaryIdentifier("file"),
     AdditionalIdentifier("f"),
     CommandUsage("[<recipient>] [<filelocation>]")]
    FileTransferInitiation,

    [PrimaryIdentifier("accept"),
     AdditionalIdentifier("a")]
    AcceptFileTransfer,

    // ...
}

My problem arises when I try to allow multiple aliases to the primary command. I have attempted this two ways: by allowing duplicates of the AdditionalIdentifier attribute, or by making the constructor argument in AdditionalIdentifier a params string[].

With the former, I implemented it by decorating the attribute class with AttributeUsage and setting AllowMultiple to true. While this does indeed achieve what I'm looking for, I'm feeling like it could get really noisy really fast to have several lines of aliases, in addition to the other attributes.

The latter also works, however, it generates the compiler warning CS3016, and says that that approach is not CLS-compliant. Obviously, this doesn't necessarily stop me from still using it, but I've learned to always treat warnings as errors.

My actual question is should I ignore my objections with duplicates and just go ahead and use them, or is there some other solution that could be used?

Thank you.

+1  A: 

Personally I would go with the AllowMultiple approach: I don't think the "noise" is going to be that much of a problem unless you really have truckloads of identifiers for each command. But if you don't like that and want to stay CLS-compliant, one other solution would be to provide overloaded constructors for AdditionalIdentifierAttribute:

public AdditionalIdentifierAttribute(string id) { ... }
public AdditionalIdentifierAttribute(string id1, string id2) { ... }
public AdditionalIdentifierAttribute(string id1, string id2, string id3) { ... }

The downside is that this does limit you to a predetermined number of identifiers.

That said, CLS compliance is really only a major consideration if you are building a library that others are likely to use (and specifically from other languages). If this type or the library is internal to your application, then it's reasonable to ignore CLS compliance warnings.

EDIT: Thinking further about this, you have quite a lot of attributes on those enums. You might want to consider creating an abstract Command class instead, and exposing the identifiers, usage, etc. as properties of that class; then derive concrete types of Command which return the appropriate values from those properties. This potentially also allows you to move the handling logic into those Command objects rather than switching on the enum value.

itowlson
I suppose you are correct. Another way would to be your multiple constructor approach and use AllowMultiple at the same time, allowing for more flexibility overall. Thank you!
nasufara
+2  A: 

Why not have a single attribute with multiple properties? Have the property for the alias take a comma-separated list. This is the approach they take in MVC for things like the AuthorizeAttribute for Roles. Internally, the property parses the string into an array for ease of use in the attribute class, but it allows you an easy way to set up your configuration.

public class IdentifierAttribute
{
    public string Name { get; set; }
    public string Usage { get; set; }

    private string[] aliasArray;
    private string aliases;
    public string Aliases
    {
         get { return this.aliases; }
         set
         {
             this.aliases = value;
             this.aliasArray = value.Split(',').Trim();
         }
    }
}

Then use it like:

public enum CommandType : byte
{
     [Identifer( Name = "file", Aliases = "f", Usage = "..." )]
     FileTransferType,

     ...
}
tvanfosson
Thank you for suggesting this, I completely forgot about this possibility! This should reduce the number of classes and decorations :). If the number of duplicates does get to be a problem, I think I will use this approach.
nasufara
A: 

Yet another approach would be to have the attribute take an array of strings as a constructor parameter - that way, you get the compiler to parse the array for you (at the expense of a little more goop when applying the attribute) thus:

[Identifiers(new string[] {"Bill", "Ben", "Ted"})]

A quick 'n dirty example of implementing & using such a technique looks like this:

using System;
using System.Collections.ObjectModel;
namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            SomeClass.TellMeAboutYourself();
        }
    }
    public class Identifiers : Attribute
    {
        private string[] names;
        public Identifiers(string[] someNames)
        {
            names = someNames;
        }
        public ReadOnlyCollection<string> Names { get { return new ReadOnlyCollection<string>(names); } }
    }
    [Identifiers(new string[] {"Bill", "Ben", "Ted"})]
    static class SomeClass
    {
        public static void TellMeAboutYourself()
        {
            Identifiers theAttribute = (Identifiers)Attribute.GetCustomAttribute(typeof(SomeClass), typeof(Identifiers));
            foreach (var s in theAttribute.Names)
            {
                Console.WriteLine(s);
            }
        }
    }
}
Dan Blanchard
+1  A: 

You could also use "params string[] aliases" in the constructor to allow a variable argument list:

[AttributeUsage(AttributeTargets.Method)]
class TestAttribute : Attribute
{
    public TestAttribute(params string[] aliases)
    {
        allowedAliases = aliases;
    }

    public string[] allowedAliases { get; set; }

}

This would allow you to do:

[Test("test1", "test2", "test3")]
static void Main(string[] args)
RussTheAerialist
+1 prettier than mine!
Dan Blanchard