3 ms·
Not sure what the point of the implicit operator overloads was. Why not just have: public class CreateBooking { public int TypeId { get; private set; }
by philbarr 12y ago
Not sure what the point of the implicit operator overloads was. Why not just have:
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.
- npinguy 12y agoYeah, not sure how this is rising, this approach is much simpler. 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...
- danbruc 12y agoI would prefer static read-only properties over methods and always return the same instance. 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.
- kevingadd 12y agoIf the field is readonly why did you make the property in the first place? Just do public static readonly BookingTypeId Gold. It's equivalent.
- Locke1689 12y agoIf this is public surface area, the change from field to property is a binary break.
- danbruc 12y ago...and you never know when you might have to add something to the getter. It's the same reason why you usually prefer properties over public fields even if there is no logic in the accessors. Similarly you usually want to expose constants by wrapping them in a read-only property because otherwise the compiler will inline the constant and you can not change it without recompiling all dependencies.
- klauserc 12y agoIt's a struct. It will always be returned by value. Furthermore, the struct has a single Int32 field. The code generated for "new BookingTypeId(3)" is about the same as for "3".
- pradocchia 12y agoNot sure what the point of the implicit operator overloads was. 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.