Awesome! Got some nice feedback and questions on a previous post of mine, and got to think things over about some design decisions, getting the possibility to change the API before going live. Thanks! Now lets address some of the reoccurring questions.

First. Remember. The point is about making the API discoverable and open but at the same time encapsulated.

Do you cover this structure with tests?

Yes. Why? Well, in the actual case of the domain, it is about money transactions, hence one wrongly configured value will cause the consumer unwanted side effects which could lead to bad will.

Why not an enum?

An enum is static. It can not represent a dynamic custom value. Hence if the backend accepts a new option before the SDK is updated, enums fail.

Any possible future functionality to the “entity”, in the case of an enum, would need to be added as extension methods, which in this case is nothing I want.

Even though the underlying value is an int, working with enums introduces an explicit cast, which of course I can encapsulate, so this is not a big hindrance.

Why not a static class with constants (key-master)?

Key-master is probably a bad name. It has nothing to do with the movie “Ghostbusters”. More: I’m the master of all possible key-values. Otherwise, this construct suffers from the same issues as enums do. It’s static. Also, it can not be taken as an argument to a constructor and the BookingTypeId entity concept is taken away, hence functionality can not be added.

public static class BookingTypeIds {
    public const int Gold = 3;
    public const int Silver = 2;
    public const int Bronze = 1;
 }

Why construct items using factory methods and not fields or properties?

Regarding fields, I try to avoid fields when they are exposed outside the class. Why? If I need a property later on, it’s just a simple change, right? Yes. Same name and type but it still brakes backwards binary compatibility. Now, the chances in this case that someone should be fiddling with reflection against it should be very, very small, but even so, I would then go with properties.

The rest is a bit harder to explain which probably is a good sign of a bad choice from me and is also the one I might consider refactoring to. But, yes, properties can be used in a case like this, when no side effects or long running processes are involved. In this case, there clearly isn’t any long running processes involved. How about side effects? Well an argument exception can be thrown if the value is out of accepted range. You remember? The line: //...some simple validation logic....

private BookingTypeId (int value) {
    //...some simple validation logic...
    _value = value;
}

How-ever, in this case I’m in the control of the generation of the possible values: “Gold”, “Silver” & “Bronze”; so causing an exception to be thrown is something I can “look aside” from.

Then why not properties? Consistency? Exposing the constructor of BookingTypeId is not an option, because people will then miss the possible preconfigured values (which could be solved by letting it take an enum) and instead be using custom values that in 99.9% matches the pre-configured ones. So “Consistency” in this case would refer to the factory method “Custom”:

public static BookingTypeId Custom(int value) {
    return new BookingTypeId(value);
}

But shouldn’t this be named “Create” or something? Well, yes, probably. Just didn’t want: “CreateGold, CreateSilver, CreateBronze”; hence it became “Custom” to match this convention.

But yes, I agree, properties are probably a better way, BUT I (for now) would like to return the same value each time but different instances, and that is something I think a method expresses better, even though we are used to DateTime.Now.

Why implicit operator? Why not expose the inner value instead?

Because it’s geeky? Jokes aside. Yes, I agree that most people that come to the point to maintain the code probably need to scratch their head a bit, which is not a good thing. How-ever, in this case the actual entity is the value. Take DateTime. It represents an entity “DateTime”. Do you see a Value property on it? The idea is that BookingTypeId is more than an int. Currently, e.g. in the form of being as simple as argument range validation check. So with this in mind, that’s why it’s for now is not exposing the value.

Summary

Not much to say, except that in every design process we have different decisions to take and I’m grateful for all the feedback. Will probably lead to a bit of changes.

Cheers,

//Daniel

Category:
C#, Development
Tags:

Join the conversation! 3 Comments

  1. A few comments:

    .NET enums are odd beasts. Unfortunately, they are *not* statically restricted to just the values in the enum; those are merely “convenience” values that are easily accesible. It is perfectly valid and well-defined to cast any arbitrary int(*) to an enum. In other words: you can have custom booking IDs in your enum. Should you do this? Probably not.

    On binary compatibility: I’ve seen this listed as a best-practice before, and I think that’s not a good idea. Binary compatibility is almost almost irrelevant: almost nobody replaces a dependency without a recompile. This is really only relevant if you’re making a GAC-installed redistributable class library; and the only common example of that is .NET’s own class libraries. Most people aren’t in the business of writing class libraries; and if they are they’re usually distributed to (web)application developers who package them with the rest of the app (and thus aren’t updated seperately from the app itself). Limiting yourself to binary compatibility means restricting access to potentially useful compiler features and checks – don’t do it unless you’re actually getting something in return.

    On reflection: I’ve written lots of reflection code; and it’s not hard to accept fields or properties. Fields are faster; and they’re more communicative to the developer precisely because they’re more restrictive – as such I’d pick those any day for an api unless I actually need that flexibility. Furthermore, with reflection, even things like the order of method + field declarations, and private members become accessible – where do you draw the line? Fortunately, it’s rare for code to go arbitrarily rummaging behind the scenes with reflection, but that also means that you tend to know when your type is exposed to reflection-based consumers and usually what those are. Typical scenarios are things like ORM’s and serialization helpers. Some of these don’t support fields (though others do). Even when that’s the case, however, you’re still safe picking fields over properties initially: after all, that just means your class’s “internals” (the field) aren’t accessible to that reflection-based consumer. If and when that is an issue, you can always make the switch; that’s essentially equivalent to a code change that adds a member and that’s usually just fine. In any case: I don’t think it’s a good idea to limit your API choices based on potentially restrictions that highly dynamic code might have: the whole strength of a statically typed language such as C# is that the tooling can help you keep control of your codebase under certain (statically known) restrictions. Circumventing those by means such as reflection is not be the norm. Let me put it another way: I’ve used C# almost from its inception including lots of reflection and code generation, and though I’ve encountered lots of places where properties were necessary, they’ve all been immediately obvious either at compile time (due to interfaces) or during even the most superficial ad-hoc testing. By contrast, surprising side-effects of properties have bitten me on occasion, and similarly, surprising mutation of a value has too. Readonly fields prevent both problems, and that’s a lot more valuable than the fairly hypothetical case that code you didn’t expect actually used reflection to depend on the distinction between a field and a property *and* you didn’t notice this during testing.

    On methods & instances: `BookingTypeId` is a struct. The notion of “instance” doesn’t really apply. Both a readonly field and a method returning a “new” object have the same semantics – just the value returned. I mean, you could construct the integer 0 using “new int()”, but there’s no reason to – since int is a value type, neither returns a reference to an instance.

    Reply
    • Thanks for the input and feedback. I probably missed something in it ;-)

      Aware of that enums can have another base, fully irrelevant in this case though. Did not know that you actually can assign a value that is not part of the enum. Not sure when someone would use this (and haven’t given it any thoughts either). Agree on them being *odd beast*.

      Not disagreeing on that it doesn’t increase the complexity to support both fields/properties in reflection.

      Not referring to it as an instance of a reference type, but an instance of that value-type that will be created by copying the values upon assignment. The reason for new in this case, is that I want the inner field to be readonly, hence assigned via constructor. Yes, I still can use the default constructor, but I can not assign the inner value.

      When it comes to properties vs fields I, in general, still prefer properties over fields when it comes to public APIs. Fields for me signals that “this is a internal thingiee” and if I need read-only, I would still expose it via an getter property.

      Thanks,

      //Daniel

      Reply
  2. (*): enums are backed by ints by default, but you can choose a different “base” class in which case any of the base-class’s values can be coerced to the enum type.

    Reply

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Follow

Get every new post delivered to your Inbox.

%d bloggers like this: