Freigeben über


Coding Styles: bools, structs, or enums?

I often find myself writing or consuming API's which require the caller to specify some sort of options.  I've seen numerous ways to specify those options, but I've yet to find one that I really like.  Let's work with an example throughout the rest of this post.  Imagine we're working on an application which has some notion of a User.  We're now about to create a function to output a User to a Stream.  We want to provide two options:

  1. Should we include the User's email address?
  2. Should we include the User's phone number?

I can imagine several ways of designing this API.  First, we could use a couple of bool parameters, like this:

 

 void DisplayUser(User user, TextWriter stream, bool displayEmail, bool displayPhoneNumber)

Next, we could use a flags enum to represent the options, like this:

 [Flags]enum DisplayUserOptions{    Email = 0x1,    PhoneNumber = 0x2,    Both = (Email | PhoneNumber),}void DisplayUser(User user, TextWriter stream, DisplayUserOptions options)

Finally we could create a struct which contains two bools instead of the enum, like so:

 struct DisplayUserOptions{    public bool Email;    public bool PhoneNumber;}
 void DisplayUser(User user, TextWriter stream, DisplayUserOptions options)

I don't like passing the two bools, because I find that reading the callsite of a method designed like this to be difficult, because you don't know what the parameters mean anymore.

 DisplayUser(user, Console.Out, true, false);

Does this display the email address or the phone number?  I can't remember anymore.  I know that parameter help can tell you the names of the parameters, but that doesn't help when I'm doing a code review in windiff, since windiff doesn't have parameter help tool-tips yet.

On my current team, we have a convention that when we call a method like this, we put the parameter name in a comment, so that we can see it, like:

 DisplayUser(user, Console.Out, true /*displayEmail*/, false /*displayPhoneNumber*/);

However, in general, I don't like to rely on conventions that force people to put comments in a certain style to make the code readable.  If possible, I'd rather design the API in such a way that it has to be readable.

The second option is using enum flags to represent the options.

 DisplayUser(user, Console.Out, DisplayUserOptions.Email);

My problem with this approach is that it is hard to get right, both for the caller of the API, and the implementer.  This example is sort of trivial, but I can remember a time when I was dealing with a bit-field that contained 31 unique bit values that could be set and unset independently.  Getting all of the ~ and | and &'s just right was very hard, and once it was done, it was hard to figure out what it was trying to do.  A final reason that I don't like enums is that in practice I often find that there are more behaviors that I want to be able to add to the options which isn't possible with enums.  For example, it might be a requirement that two of the options are mutually exclusive.  It's difficult to ensure that this is enforced with an enum.

Finally, we have the option of using a struct, which is addresses both of the two concerns above.  You can write a call like:

 

 DisplayUserOptions options = new DisplayUserOptions();options.Email = true;options.PhoneNumber = false;DisplayUser(user, Console.Out, options);

Well, that's certainly clear.  It also makes it easier to understand control flow that sets or clears the options, and to understand conditions based on them.  It also gives a place to add that behavior: I can add methods to the struct, make the fields into properties which have setters that do validation, etc.

The problem with this approach is that in simple cases like the above, it is much more verbose than either of the other two alternatives.  However, I recently realized that in C# 3.0, we can take advantage of Object Initializers to make the simple case simple again:

 DisplayUser(user, Console.Out, new DisplayUserOptions { Email = true, PhoneNumber = false });

It's almost like having named parameters in C# :)

What do you think?  Which alternative do you prefer?  Do you have another one that I haven't thought of?

Comments

  • Anonymous
    November 30, 2007
    The comment has been removed

  • Anonymous
    November 30, 2007
    The first option with a set of boolean parameters is a bad idea, not just for readability, but for extensibility.  If you add additional options in a later revision, you would have to change the method signature, which would result in a breaking change to your API (internal or external, it's still undesirable). As for options 2 and 3, using an Enum would be compatible with .NET 1.0, 1.1, 2.0, and 3.0, but object initializers would only work with .NET 3.5.  Having said that, I think that using an object with the appropriate fields is better than an Enum, since you could abstract the initialization into a function to improve readability (I'm thinking of your 31 options scenario here, which would also make the object intializers convention formatting a challenge).

  • Anonymous
    November 30, 2007
    I pretty much agree with your take on this. The first option, bools, doesn't really have any benefits, but does have a lot of drawbacks. Enums are nice for relatively simple situations (like your example), while using a settings object (be it a class or a struct) works well for more complex scenarios.

  • Anonymous
    November 30, 2007
    I frequently use *Options structures with nullable types representing optional parameters.

  • Anonymous
    November 30, 2007
    With the anonymous types feature of C# 3.0, can't DisplayUser(user, Console.Out, new DisplayUserOptions { Email = true, PhoneNumber = false }); be shorteneed to?: DisplayUser(user, Console.Out, new { Email = true, PhoneNumber = false }); Seeing as though the DisplayUserOptions type's only purpose is to provide named arguments to the DisplayUser function, it doesn't seem important to keep the class name there.   Then again, you could argue this reduces the clarity of the function call for later programmers, but, assuming DisplayUserOptions is a sealed class, the documentation for the DisplayUser function should make it pretty clear what the type of the third argument is.

  • Anonymous
    November 30, 2007
    Great use of object initializers. Will be even nicer when C# supports initializing readonly structures with that kind of syntax...

  • Anonymous
    November 30, 2007
    Bruce, unfortunately, you can't omit the type name in this scenario.  The reason is that the method needs to take a named type, and there isn't a way to do that.  Anonymous types don't unify to named types, even if they have the same set of property names.

  • Anonymous
    November 30, 2007
    Down with premature optimization!  Up with readability & obvious correctness. I've created too many bugs in my career because of bit twiddling. That was a vote for the structs of bools.

  • Anonymous
    December 02, 2007
    I prefer the following: DisplayUserOptions options = new DisplayUserOptions(); options.Email = true; options.PhoneNumber = false; DisplayUser(user, Console.Out, options); However, I do not care for Object Initializers in C# 3. To me, the verbosity of the two are the same because they compile into the exact same code. It's nothing but syntactic sugar. There are also several advantages to not using object initializers:

  1. Can more freely add comments
  2. Easier to debug The boolean option is the worst in regard to both readability and (perhaps more importantly) extensibility.
  • Anonymous
    December 03, 2007
    Hi Craig, I understand that Object Initializers compile to the same IL, and so their performance and semantics are identical, but that doesn't mean they are equally verbose.  By verbosity, I mean, "how much code does it take to express the idea."  C# has several other constructs which are purely syntactic sugar, but which are nonetheless very useful.  Some examples are:
  1. Loop constructs like while/do/for.  These are all just a series of goto's.
  2. the "using" statement, which exands to a try/finally with a Dispose call.
  3. the "foreach" statement, which expands to a try/finally, a call to GetEnumerator and a while loop.
  4. Anonymous methods, which expand to a display class plus method.
  5. Lambda's which are equivalent to anonymous methods but with less verbose syntax.
  6. Query Expressions.
  7. etc. Just because something is "syntactic sugar" doesn't mean that it shouldn't be used if it makes your code easier to read. Regarding adding comments, I agree that an object initializer inline in a call is not a good structure for adding comments.  If I needed to, I would probably apply the "introduce explaining variable" refactoring to end up with an object initializer assigned to a local: var options = new DisplayUserOptions {    // Free to add comments here.    Email = true;    PhoneNumber = false; }; DisplayUser(user, Console.Out, options); Regarding debugging.  This is a good point.  It's too bad that the VS debugger doesn't support expression level debugging, so that you could set a breakpoint and step on each of the lines of object initializer.  I hope that capability gets added in a future version of the debugger.  However, for options type structs like these, I usually find that the expressions are simple enough that I don't need to break on individual ones.
  • Anonymous
    December 09, 2007
    Welcome to the thirty-seventh edition of Community Convergence. Visual Studio 2008 has been released

  • Anonymous
    December 09, 2007
    Welcome to the thirty-seventh edition of Community Convergence. Visual Studio 2008 has been released

  • Anonymous
    December 10, 2007
    I vote for structs as well :) I also find it a good pattern to derive the struct name by adding 'Options' to whatever name the method has.

  • Anonymous
    December 12, 2007
    I don't like any of those options.  Personally, I think the clearest option would be an enum with all the possibilities, such as, say: enum DisplayUserOptions {    None,    Email,    PhoneNumber,    EmailAndPhoneNumber } Now you don't have to worry about passing in DisplayUserOptions.Email | DisplayUserOptions.PhoneNumber which sounds rather unclear when read aloud ("Email OR PhoneNumber", when you really mean the customer has "Email AND PhoneNumber").

  • Anonymous
    December 12, 2007
    The comment has been removed

  • Anonymous
    December 12, 2007
    Hi Kyralessa, I agree that having a "combination value" in the enum is probably better than the straight enum, but still has three drawbacks in my opinion:

  1. The implemenation of the "DisplayUser" becomes a little bit more complex, because you need to check combinations of flags.  However this is a minor issue from an API design point of view, since it affects the implementor, instead of the caller.
  2. It results in a cominatorial explosion of the possible values.  For example, in my case of 31 possible flag values, your enum would need to have 2^31 different values specified explicitly, which can't be good :)
  3. There is still no logical home for all of the other behaviors and validation associated with it.  In my experience, that behavior is almost always there, it's just hard to see unless you're looking for it, and you have a better place to put itt.
  • Anonymous
    December 12, 2007
    I generally follow the guidance in the .NET Framework Design Guidelines - but there is a case I think for reviewing those given the new syntactic forms that C# 3.0 enables.

  • Anonymous
    December 16, 2007
    I have been using bools in simpler situations and enums in slightly more complex ones. But I must admit that the situations I had had to code for never had 2^31 possibilities. I would think that anytime it gets more complex, use a class and assign it the responsibility of figuring out what's right and what's not. Someone suggested to enumerate all possibilities in an enum rather than be burden the consumer with providing a|b when what you mean is that a and b. The disadvantage that I see is that even though you have all the valid possibilities explicitly stated it won't stop someone from passing yourenum.a|yourenum.b & yourenum.c or any other combination to your API. How's that going to be handled? Therefore, IMO,

  • bools in academic/example sitations
  • enums in slightly complex ones
  • classes in more complex ones seems to be a solution to me.
  • Anonymous
    December 16, 2007
    I'd like to see syntax like this: public class Class1 {    public bool GetSomething(int iParam, string sParam)    {        return true;    } } Which is then called like this (analogous to the new way of calling constructors in 3.0): Class1 c = new Class1(); bool ret = c.GetSomething( iParam = 0, sParam = "you lose"); Is this what an earlier commenter referred to as "named parameters"?

  • Anonymous
    December 17, 2007
    Kenneth, yes that's named parameters.

  • Anonymous
    December 18, 2007
    I like the enum option. Regardless of what you do, when it comes to 31 different options to single method, it's gonna get complicated. Besides, if you use the struct option, the code to set all 31 fields of a struct will be more than the code to AND and OR 31 enums.

  • Anonymous
    December 18, 2007
    Tundey, I agree that the case of having 31 different options is a pathological case.  And while it may end up being more code, I think that the code for a struct based solution is easier to understand, and so I tend to prefer it, even though it might be a little bit more verbose.

  • Anonymous
    December 19, 2007
    The comment has been removed

  • Anonymous
    December 20, 2007
    Shadders, I pretty much agree with all of your points, and I frequently do use your strategy of having multiple named methods.  This does suffer from the combinatorial explosion problem however.  I'll admit that 31 options isn't a good example, but even if you have 6 or so independent options, that's still a lot of methods to maintain. The real life example where I had 31 was for determining what things to include in completion lists.  The options were things like: include static, include instance, include non-public, include properties/events/methods/types/namespaces, etc, but you're right that it isn't a very good way of structuring the code.

  • Anonymous
    December 25, 2007
    The comment has been removed

  • Anonymous
    December 25, 2007
    The comment has been removed

  • Anonymous
    January 04, 2008
    I use techniques 1 and 2 with the same concerns. There will be no proper solution unless named parameters are made part of C# --something I've long wished for. So that's my vote: "Technique 4" of a better future world: C# named parameters.

  • Anonymous
    January 06, 2008
    I prefer using the enum with params, so the call would be something like this: DisplayUser(user, Console.Out, DisplayUserOptions.Email, DisplayUserOptions.PhoneNumber); This way the caller doesn't have to know bitwise operations (for small list of options). You still may use bitwise operations to let the code more readble in a very large list of options.

  • Anonymous
    January 06, 2008
    Followin is an example of code using Enums/params/Extension methods: Using this extension method (that could be inproved with generics): public static class DisplayUserOptionsExtension {        public static Boolean In(this DisplayUserOptions option, params DisplayUserOptions[] options) {            DisplayUserOptions selectedOptions = DisplayUserOptions.None;            foreach (DisplayUserOptions selectedOption in options) {                selectedOptions |= selectedOption;            }            return (selectedOptions & option) == option;        }    } We can use a code like this to call the method: DisplayUser(user, Console.Out, DisplayUserOptions.Email, DisplayUserOptions.PhoneNumber); And the implementation of the method is not difficult either with the extension method: public void DisplayUser(User user, TextWriter textWriter, params DisplayUserOptions[] options) {            if (DisplayUserOptions.Email.In(options)) {                // print e-mail            }        }

  • Anonymous
    January 07, 2008
    I tend to agree with Ricardo. This sort of dependency injection keeps the complexity of the parameters with the parameters. This object can grow to deal with changes without affecting the calling and called function(s). I think that wherever possible complexity and or functionality should be encapsulated.  Of course this approach introduces complexity in terms of implementation but as Shadders has pointed out we reap what we sow. I often find myself in both consumer and provider roles. I am lazy. So I tend to work overly hard to minimize the work I will have to do in the future. I think of this as an investment which has the largest return in terms extensibility and containment of possible defects.

  • Anonymous
    January 07, 2008
    Riccardo, I hadn't considered the use of params enums.  It's very interesting, I'll have to think about it more.

  • Anonymous
    January 22, 2008
    Although the consumer of Ricardo's solution has a very easy syntax, it looks like this comes with a performance penalty - constructing a temporary object for a single boolean check. C++ used to use constructs like this: union DisplayOptions { struct {  bool Email:1;  bool PhoneNumber:1; }; int BitFlags; }; ...which, although a little ugly to define, gave performance + readability. Is there no C# equivalent?

  • Anonymous
    January 23, 2008
    The comment has been removed

  • Anonymous
    January 23, 2008
    The comment has been removed

  • Anonymous
    January 23, 2008
    Hey Bevan, thanks for the comment.  Your second approach looks a lot like the Fluent Interface approach that I mention in my follow up post. I agree that it's a great way too, since you can create an API that really flows together well.

  • Anonymous
    January 24, 2008
    I generally prefer enums; I have never found bitwise operations particularly complicated or burdensome, and the ease of use and readability on the consumer side is a huge plus. The only time I would move to using an options class would be if, as you pointed out in your post, there was the possibility of complex interactions between options, which is generally not very common; and even then, often this can be overcome by factoring your options into two or three enums where the individual options are addressing a common issue. I have often thought that the Reflection API's could have done much better in this area.

  • Anonymous
    January 27, 2008
    None of the above work well with Web services, where you need the caller and callee to be as independent as possible. Your client may be several versions behind the server, which is an extreme version of late binding I guess. Also callers will likely be using a different programming language. So I vote for ordinary bitmaps (not even enums).

  • Anonymous
    January 28, 2008
    @Pete: .NET enums map directly onto a bitmap for purposes of exposing the method to external callers. Why not use the enum from within .NET to make your life easier?

  • Anonymous
    January 29, 2008
    I prefer the enums anytime. Once you get a hang of how bitmasking works it's all very easy. What I usually do is use the enum as a method argument and then just declare local boolean variables for each of the enum items and initialize them with the bitmask values. For example if my enum has three items called READ=1, WRITE=2, and APPEND=4 bool read = ((byte)(enumParam & myEnum.READ)) > 0; bool write = ((byte)(enumParam & myEnum.WRITE)) > 0; bool append = ((byte)(enumParam & myEnum.APPEND)) > 0; If there are too many enum items or each value is only used once then I'd probably do the bit-masking in-place rather than declaring local variables. I would probably be casting to int or long or whatever base type my enum needs to accomodate all enum items.

  • Anonymous
    January 29, 2008
    The comment has been removed

  • Anonymous
    January 29, 2008
    Bevan: Functionally I don't see any problems with your approach but I don't think it's that readable. Functionally that's exactly the same as using properties since a property is just a method internally like C++ getter or setter methods. However .NET's OOP model uses properties for this purpose, so a .NET programmer will expect that to be done with properties so that will be rather confusing for a .NET programmer. If you must use methods for any reason you might want to use the std. convention used in cpp and other languages for getter and setters (use a get or set prefix, the set takes in a value and the get returns it. A few questions:

  1. How can you tell if the WithEmail() method has already been called for an object?
  2. Do you have a WithoutEmail() method to clear that flag? Whatever the answer is, properties make more sense to me. I just realize that here I'm discussing the use of methods (used as getter or setters) vs. using properties in .NET which functionaly is the same thing, but this post is talking about method arguments vs. method calls to set the arguments and there is a huge functional difference in that case. #1 you cannot use methods to provide values for another method unless you make the variable global which unless you really need it global for anohter reason is a very bad idea. #2 even if the method is infact an instance method and you do need to save the parameter globally for the instance it's still a bad idea to use a method call just to set a value that could be set on the next call without the need of allocating and deallocating a stack frame for that method.
  • Anonymous
    January 29, 2008
    The comment has been removed

  • Anonymous
    January 31, 2008
    Enums for the win!

  • Anonymous
    February 01, 2008
    While no size fits all, when the initialization comes to more than a few items, I like the structs.  There is nothing to say that bools (which I like) and enums (which I also like) can not live together in these structs, depending on what is being initialized. (Say one item might be a method of formating the text). When using the structs, we have the option of using object initializers, or to pre-build the struct and fill it member by member before the call using it. I also do not like the idea of having a handfull of functions to use depending on how I want to initialize.  I believe that the fewer the functions in a struct/class the easier it is to maintain and understand.  I find when I browse a struct/class that has a large function base my eyes start to glaze over. Thank you for posting this.  I find it a good thing to go back and question how we do the common tasks, and appreciate seeing how others deal with these.

  • Anonymous
    February 17, 2008
    Up to now, I only used the enum approach, but structs may be a good alternative if you have many options. But then again: I think "too many options to us an enum" is a big hint to think about the design again. Mutual exclusive options fall into this category, as well: Why not make two methods, instead? In case you go for the complex option parameter, another struct approach might be handy to be able to start with an enum first and migrate to stuct later: struct DisplayUserOptions {    static readonly DisplayUserOptions Email = new DisplayUserOptions(1);    static readonly DisplayUserOptions PhoneNumber = DisplayUserOptions(2);    private DisplayUserOptions(int bits) { ... }    // overload &, | operators ... } (I did not try this in real code, just a thought...)

  • Anonymous
    February 18, 2008
    It all depends on the complexity. Nothing else. The complexity determines the best suitable method. A boolean parameter would be used in conjunction with a method that is named in such way that it is obvious what the boolean parameter does: MyObject.Reset(true); MyObject.Reset(CheckBoxClearSettings.Checked); The next step of specifying options is the enum, naturally. When options are exclusive, the enum IS best suited for the option. MyReport.Export(ExportTo.PDF); MyReport.Export(ExportTo.File, fileName); The other situation is the example used in this topic. We're now speaking of "Flags" DisplayUser(user, DisplayUserFlag.EMail|DisplayUserFlag.Name); This however would only be feasible with a limited number of flags, where there is no interdependancy on flag bits. The next level is using a class. This way obviously offers the most flexible way of specifying parameters. public class MyMethodDisplayOptions {    public bool ShowEmail;    public bool ShowAddress;    public bool ShowCity; } enum ExportTo {PDF, Text, Printer......} public class MyMethodOptions {    public MyMethodDisplayOptions DisplayOptions { get.... }    public ExportTo ExportTo { get .... } } The goal is to avoid having a method incredibly long parameter lists. It doesn't read well, code doesn't look smooth. It may take some effort to define the parameter support objects, but it is very readable. It also offers the possibility to embed intelligence into the options classes, where they can adjust options when other options are specified. (Mutual exclusiveness can be enforced etc.) Even exceptions can be raised by the options classes. So... in the end, there's no "best" way by itself. I use all three ways of specifying options. It's just a matter of picking the right way for a given situation. The thing to do is to determine how you can keep the method parameter lists short. Where option names, enums or classes are self-explanatory. Happy coding :)

  • Anonymous
    February 20, 2008
    This is another vote for method objects, although the technique is a little obscure at the moment it's got a lot going for it in terms of discoverability, readability & flexibility. That said, it does take a lot to setup, so unless they're really needed, I'd go with named parameters if they're available - although no projects I'm on can use them at the moment.

  • Anonymous
    February 28, 2008
    How about the move method to another class refactoring?  Create a separate renderer or view class for each domain entity / target output combination. public class UserTextStreamRenderer : UserRenderer {   public User UserData {get;set;}   public bool ShowEmail {get;set}   public bool ShowPhone {get;set}   //other options...   public TextWriter TargetStream {get; set;}   public override void Render(){Render(TargetStream);}   public void Render(TextWriter stream); } public abstract class UserRenderer {   public abstract void Render(); }

  • Anonymous
    February 28, 2008
    The comment has been removed