Using implicit operator overloads to keep your C# API discoverable and open
danielwertheim.se
danielwertheim.se
There are very few cases where you should have implicit conversions. Implicit conversions basically imply that the two types are so similar that no possible semantic difference can be observed after the conversion. For example, int is implicitly convertible to long because there can be no possible information loss and almost every case where long is allowed, an int would also be allowed in that place. Even overload resolution works correctly because the overload is first tried without the implicit conversion.
Enums (or other structs), on the other hand, can easily have different semantics. For example, if the enum values are auto-assigned and you add a value into the middle of the enum, all subsequent values are now offset-increased by 1. If you then switch on the int values, but not the enum values, your semantics will change.
Here an explicit cast is a good thing -- it's saying to the compiler that I'm doing something potentially dangerous, but just trust me on this one.
public class CreateBooking {
public int TypeId { get; private set; }
...
public CreateBooking(BookingTypeId typeId, ..., ...) {
TypeId = typeId.Value;
...
}
}
public struct BookingTypeId {
private readonly int _value;
private BookingTypeId (int value) {
//...some simple validation logic...
_value = value;
}
public int Value { get { return _value; } }
public static BookingTypeId Custom(int value) {
return new BookingTypeId(value);
}
public static BookingTypeId Gold() {
return new BookingTypeId(3);
}
public static BookingTypeId Silver() {
return new BookingTypeId(2);
}
public static BookingTypeId Bronze() {
return new BookingTypeId(1);
}
}
With the added bonus that you're using less fancy stuff, so you don't need to explain to the junior dev that gets to maintain it what's going on.I could understand if he created an implicit cast from int TO BookingTypeId. This way, at least, consumers could not care about BookingTypeId if they didn't want to and pass in an int directly. But that has it's own whack of problems...
private static readonly BookingTypeId gold = new BookingTypeId(3);
public static BookingTypeId Gold
{
get { return BookingTypeId.gold; }
}
It becomes a bit inconsistent because of the method for the custom case but I would accept that for less noise in the common case.Furthermore, the struct has a single Int32 field. The code generated for "new BookingTypeId(3)" is about the same as for "3".
I assume he didn't want to change the signature of the constructor for whatever reason, but still provide a small measure of discoverability (and tractability). Such things do happen.
In that case, I might have used an enum plus a second constructor to wrap the explicit conversation. Maybe that didn't occur to him, or maybe he had other constraints.
An enum IS the right solution here. Its more maintainable because everyone who touches the code base will implicitly know how the code works. With the solution he used, as i'm reading the code... i'm going to go "oh hey, huh i wonder how that works", then i'll spend 10 minutes trying to understand the code, added to the extra 10 minutes probably spent writing this code, I think its a bad idea.
if you're worried about passing a bad value in still, i think its fine to add a range check in the constructor, and to throw an exception. Exceptions are not something to be afraid of.
Also, i don't know what library he's using... but i feel somewhat strongly like i've done something like this in the past where i've serialized JSON, and enums have serialized as integers (as long as entry assigned a numeric value?). So it wouldn't be an issue to pass it in as a enum anyways.
But doesn't this implementation kind of defy the purpose of having an enum-like functionality in the first place?
The static quality of enums is only one of their features, and sometimes a hindrance, and it good to have other options.
I see "here is something that looks like an enum, and its possible values are gold, silver and bronze. But you can also give 10, 42 or any number as a value and it will still work".
I don't mean to be overly critical (clearly this approach has worked well for the author), but I am a bit unsure about the perceived benefits.
I might have used an enum plus a second constructor to wrap the explicit conversation. I guess it depends on what's going on in the rest of the API.
Can someone help me understand what he means here? Thanks
enum Test
{
A,B,C
}
int x = Test.A; // This won't assign
int x = (int)Test.A; // This will
> and they are staticNot sure why that's important.
> Also, any logic to them, would have to be added as extension methods
You can extend enums using extension methods, just like you can with classes or structs (you can also extend delegates FYI).
> and in the case I had, logic tied to the BookingTypeId was arround the corner.
No idea.
The OP wants the first one to work (that requires an implicit cast). The second one is an explicit cast. You explicitly demand a conversion to int.
> > and they are static > > Not sure why that's important. You cannot add more values to the enum without modifying the enum. If you don't own code to the Assembly that defines the enum, you're out of luck. With the OP's solution, a third-party dev could add more 'constants' to "TypeId", simply by providing more factory methods.
Sure those factory methods would not be located on the "TypeId" type, but that's acceptable.
>> Also, any logic to them, would have to be added as extension methods
> You can extend enums using extension methods, just like you can with classes or structs (you can also extend delegates FYI).
Yes, but you can't add properties, for instance. Also extension methods are a C# and VB compiler feature. Other .NET languages (especially dynamic ones) will not discover these methods. Same goes for uses of 'dynamic' and reflection.
type TypeId = TypeId of int
with
static member Gold = TypeId 1
Custom is now just "TypeId 123". Otherwise, TypeId.Gold. (This is assuming you don't want to specify the cases directly as union cases).You can also abstract this to a general phantom type like "type Id<'a> = Id of int". Now functions can ask for an Id<'a> for arbitrary types. Sort of like a poor version of the units-of-measurement.
This article's kind of boilerplate OO stuff seems like it's out of control. Over the past week, I've been using open source clients in .NET to connect to social media services. And a common thread is to write oh-so-much code and provide tons of types just to get a simple job done.
One Twitter client lib requires 7 binaries. Types are split across around 50 namespaces. And of course, every type has its own file.
If I were solving the same problem:
public void CreateBooking( int chosenBookingType,... ){ ...
- and -
public string[] GetAvailableBookingTypes(){ ...
or something similar.
Your implementer can call GetAvailableBookingTypes at run-time to populate whatever interface lets you select one, then pop the index of the selection back into CreateBooking. I have it returning an array of strings as a very simple solution; you can just as easily return an array of objects that better represent your booking types (containing descriptions, and URIs to icon images or colors or whatever). The methods are named in such a way as to lend to the fact that they are related. No enums, no wonkiness and you can add new booking types at will without having to let your implementer know something changed. If your implementers are not catching on that the two methods are related, you can solve that problem with easy to read documentation.
Being as that the article consistently refers to these being fixed values, and that they're optimising for 3rd party use of their library, that doesn't seem to be the case.
This seems to be the classic case of a solution looking for a problem.
Thanks,
//Dan