3
\$\begingroup\$

I have network printers which have different attributes (e.g. supported network protocols, languages, status, print modes).

public abstract class PrinterAttribute
{
  protected PrinterAttribute(int value, string dsc)
  {
    this.Value = value;
    this.Dsc = dsc;
  }

  protected PrinterAttribute(int value, string dsc, Enum type) : this(value, dsc)
  {
    this.Type = type;
  }

  protected int Value { get; private set; }
  protected string Dsc { get; private set; }
  protected Enum Type { get; private set; }
}

And finally I will have number of classes like NetworkProtocolAttribute, PrinterModeAttribute.

public class NetworkProtocolAttribute : PrinterAttribute
{
  public NetworkProtocolAttribute(int value, string dsc) : base(value, dsc) {}
  public NetworkProtocolAttribute(int value, string dsc, ProtocolType protocolType) : base(value, dsc, protocolType) {}

  #region 1. way - get desired network protocol from locating it in all possible ones
  // all possible network protocols that a printer can support
  private static IList<NetworkProtocolAttribute> allPossibleProtocols;

  private static IList<NetworkProtocolAttribute> AllPossibleProtocols
  {
    get
    {
      if(allPossibleProtocols == null)
      {
        allPossibleProtocols = new List<NetworkProtocolAttribute>()
        {
          new NetworkProtocolAttribute(0, "None"),
          new NetworkProtocolAttribute(1, "FTP"),
          new NetworkProtocolAttribute(2, "LPD"),
          new NetworkProtocolAttribute(4, "TCP"),
          new NetworkProtocolAttribute(8, "UDP"),
          new NetworkProtocolAttribute(0x10, "HTTP"),
          new NetworkProtocolAttribute(0x20, "SMTP"), // simple mail transfer protocol
          new NetworkProtocolAttribute(0x40, "POP3"),
          new NetworkProtocolAttribute(0x80, "SNMP"), // simple network management protocol
          new NetworkProtocolAttribute(0x100, "Telnet"),
          new NetworkProtocolAttribute(0x200, "Weblink"),
          new NetworkProtocolAttribute(0x400, "TLS"),
          new NetworkProtocolAttribute(0x800, "HTTPS")
        };
      }

      return allPossibleProtocols;
    }
  }


  public static NetworkProtocolAttribute SMTP_FirstWay
  {
    get
    {
      // don't like it because you locate it by value
      return AllPossibleProtocols.Single(x => x.Value == 0x80);
    }
  }
  #endregion


  #region 2. way - create public static NetworkProtocol for every possible protocol
  private static NetworkProtocolAttribute smtp;
  public static NetworkProtocolAttribute SMTP_SecondWay
  {
    get
    {
      if( smtp == null)
      {
        smtp = new NetworkProtocolAttribute(0x80, "SNMP");
      }

      return smtp;
    }
  }
  #endregion

  #region 3. way - create enum and find it by enum
  public enum ProtocolType : int
  {
    None,
    FTP,
    LPD,
    TCP,
    UDP,
    HTTP,
    SMTP,
    POP3,
    SNMP,
    Telnet,
    Weblink,
    TLS,
    HTTPS
  }

  private static IList<NetworkProtocolAttribute> allPossibleProtocols3;
  protected static IList<NetworkProtocolAttribute> AllPossibleProtocols3
  {
    get
    {
      if (allPossibleProtocols3 == null)
      {
        allPossibleProtocols3 = new List<NetworkProtocolAttribute>()
        {
          new NetworkProtocolAttribute(0,    "None", ProtocolType.None),
          new NetworkProtocolAttribute(1,    "FTP", ProtocolType.FTP),
          new NetworkProtocolAttribute(2,    "LPD", ProtocolType.LPD),
          new NetworkProtocolAttribute(4,    "TCP", ProtocolType.TCP),
          new NetworkProtocolAttribute(8,    "UDP", ProtocolType.UDP),
          new NetworkProtocolAttribute(0x10, "HTTP", ProtocolType.HTTP),
          new NetworkProtocolAttribute(0x20, "SMTP", ProtocolType.SMTP),
          new NetworkProtocolAttribute(0x40, "POP3", ProtocolType.POP3),
          new NetworkProtocolAttribute(0x80, "SNMP", ProtocolType.SNMP),
          new NetworkProtocolAttribute(0x100, "Telnet", ProtocolType.Telnet),
          new NetworkProtocolAttribute(0x200, "Weblink", ProtocolType.Weblink),
          new NetworkProtocolAttribute(0x400, "TLS", ProtocolType.Telnet),
          new NetworkProtocolAttribute(0x800, "HTTPS", ProtocolType.HTTPS)
        };
      }

      return allPossibleProtocols3;
    }
  }

  public static NetworkProtocolAttribute GetProtocol_ThirdWay(ProtocolType protocolType)
  {
    NetworkProtocolAttribute match = AllPossibleProtocols3.Where(x => x.Type.Equals(protocolType)).SingleOrDefault();

    return match;
  }
  #endregion

And finally how would one get desired supported protocol by network printer:

class Program
{
  static void Main(string[] args)
  {
    // 1. way
    NetworkProtocolAttribute protocol1 = NetworkProtocolAttribute.SMTP_FirstWay;

    // 2. way
    NetworkProtocolAttribute protocol2 = NetworkProtocolAttribute.SMTP_SecondWay;

    // 3. way
    NetworkProtocolAttribute protocol3 = NetworkProtocolAttribute.GetProtocol_ThirdWay(
     NetworkProtocolAttribute.ProtocolType.SMTP);

    Console.ReadKey();
}

What I like about 1. and 3. way is that you see a list of all possible protocols. 3. way also has just one method to get protocol, while 2. way has one method for each protocol.

\$\endgroup\$

closed as off-topic by yuri, esote, 1201ProgramAlarm, Mast, IEatBagels May 21 at 15:08

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – yuri, esote, 1201ProgramAlarm, Mast, IEatBagels
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Why is SMTP called SNMP? \$\endgroup\$ – 200_success May 18 at 20:28
  • 1
    \$\begingroup\$ @200_success those two are two different protocols. SMTP = simple mail transfer protocol and SNMP = simple network managament protocol \$\endgroup\$ – broadband May 19 at 10:19
  • 2
    \$\begingroup\$ This looks like some kind of custom enum type. What problem are you trying to solve that cannot be addressed by using a simple enum? Also, why is almost everything of interest protected instead of public? \$\endgroup\$ – Pieter Witvoet May 19 at 14:20
1
\$\begingroup\$

Custom vs. normal enum

This looks like some kind of custom enum type, but in its current form it's not very useful:

  • All properties in PrinterAttribute are protected, which means that only derived classes can access them. That's not very useful for other code.
  • A property of type Enum is cumbersome to use because it's too weakly typed. It only tells you that it's an enum, but not which one, so you still don't know which values are valid.
  • Enum values can already be given a specific numeric value, so you don't need that Value property.
  • NetworkProtocolAttribute.FTP == new NetworkProtocolAttribute(1, "FTP") fails because they're not the same instance. The same goes for calling Equals. You'll want to implement the relevant operators, as well as override the standard Equals method (and with that, also GetHashCode). Implementing IEquatable<T> is also a good idea.

Have you considered using a normal enum instead?

[Flags]     // Indicates that this enum is intended to be used as a bitmask
public enum Protocol    // No need to specify int as underlying type
{
    None =  0x00,
    FTP =   0x01,
    LPD =   0x02,
    TCP =   0x04,
    UDP =   0x08,
    HTTP =  0x10,
    SMTP =  0x20,
    // and so on
}

// Use:
var protocol = Protocol.FTP;
protocol.ToString();  // -> "FTP"

var protocols = Protocol.FTP | Protocol.UDP;
protocols.HasFlag(Protocol.UDP);  // -> true

Enum.GetValues(typeof(Protocol)); // -> Protocol[] { None, FTP, LDP, ... }

You could add [Description("...")] attributes to these values if you really need to, but the enum names themselves are already descriptive enough. If you're using such descriptions for display purposes then that's better done at the UI layer (due to things like localization).

Other notes

  • Don't use IList<T> for static read-only properties: it allows other code to modify your list. Use IEnumerable<T>, IReadOnlyCollection<T> or IReadOnlyList<T> instead. Additionally, use an actual ReadOnlyCollection<T> (List<T> has a convenient AsReadOnly method for that).
  • Properties can be initialized directly: Foo MyProperty { get; } = new Foo();, so you don't need to explicitly define a field and perform a null-check in your getters. That null-check approach isn't thread-safe, bytheway, so if you really do need lazy initialization, use Lazy<T> or LazyInitializer instead.
  • NetworkProtocolAttribute protocol1 = NetworkProtocolAttribute.SMTP_FirstWay; can be shortened to var protocol1 = NetworkProtocolAttribute.SMTP_FirstWay;. This lets the compiler infer the type, so you don't need to write out types twice if it's already obvious from the right-hand side.
  • .Where(...).FirstOrDefault() can be simplified to .FirstOrDefault(...).
  • Personally I don't like to use this.. Properties are written in PascalCase, parameters in camelCase, so Value = value; is clear enough for me.
  • Properties that are meant to be read-only don't need a setter.
  • Just write Description out full - readability is important, especially in the long run, and writing doesn't take that much time with the help of auto-completion tools.
  • The names of your classes suggest that they're attributes - special types that are used to attach meta-data to other code. Because you're not inheriting from System.Attribute, that's apparently not what you intended, so you may want to pick different names to avoid confusion.

Finally, with regards to the linear lookups in #1 and #3, you can get rid of those by inverting the relationship between the list and the other properties:

public static NetworkProtocolAttribute FTP { get; } = new NetworkProtocolAttribute(1, "FTP");
public static NetworkProtocolAttribute LPD { get; } = new NetworkProtocolAttribute(2, "LPD");
// and so on...

public static IReadOnlyList<NetworkProtocolAttribute> AllProtocols { get; }
    = new List<NetworkProtocolAttribute> {
        FTP,
        LPD,
        // and so on...
    }.AsReadOnly();

But again, unless you have some specific requirements, I would just use normal enums.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.