Strongly typed Guid as generic struct

Question

I already make twice same bug in code like following:

void Foo(Guid appId, Guid accountId, Guid paymentId, Guid whateverId)
{
...
}

Guid appId = ....;
Guid accountId = ...;
Guid paymentId = ...;
Guid whateverId =....;

//BUG - parameters are swapped - but compiler compiles it
Foo(appId, paymentId, accountId, whateverId);

OK, I want to prevent these bugs, so I created strongly typed GUIDs:

[ImmutableObject(true)]
public struct AppId
{
    private readonly Guid _value;

    public AppId(string value)
    {            
        var val = Guid.Parse(value);
        CheckValue(val);
        _value = val;
    }      

    public AppId(Guid value)
    {
        CheckValue(value);
        _value = value;           
    }

    private static void CheckValue(Guid value)
    {
        if(value == Guid.Empty)
            throw new ArgumentException("Guid value cannot be empty", nameof(value));
    }

    public override string ToString()
    {
        return _value.ToString();
    }
}

And another one for PaymentId:

[ImmutableObject(true)]
public struct PaymentId
{
    private readonly Guid _value;

    public PaymentId(string value)
    {            
        var val = Guid.Parse(value);
        CheckValue(val);
        _value = val;
    }      

    public PaymentId(Guid value)
    {
        CheckValue(value);
        _value = value;           
    }

    private static void CheckValue(Guid value)
    {
        if(value == Guid.Empty)
            throw new ArgumentException("Guid value cannot be empty", nameof(value));
    }

    public override string ToString()
    {
        return _value.ToString();
    }
}

These structs are almost same, there is a lot of duplication of code. Isn't is?

I cannot figure out any elegant way to solve it except using class instead of struct. I would rather use struct, because of null checks, less memory footprint, no garbage collector overhead etc...

Do you have some idea how to use struct without duplicating code?

Monkeypot

Interesting need. I have a question. Leaving the code duplication aside, when you wanted to create one of those structs, you must initialize them with something. How do you make sure they are initialized with the right thing? As in..var appIdStruct=new AppId(paymentId) should not compile – Rictus

I cannot guarantee it. I cannot check everything. Even then space for bugs will decrease a bit. – Monkeypot

From that point of view, I think the space for bugs is the same, you are only moving the source to a different place. Also, you are complicating things which will actually increase that space long term. Are you planning to allocate lots of those? Will they live long in memory? – Rictus

Not much, but null checks are also good reason for structs. – Monkeypot

Have you considered creating a Params Class like FooParams and passing it to the function, and then if you are really using meaningful variable names, a misassignment would stand out and would be easy to catch like fooParams.PaymentId = accountId; – Hedwighedwiga

Answer 1

First off, this is a really good idea. A brief aside:

I wish C# made it easier to create cheap typed wrappers around integers, strings, ids, and so on. We are very "string happy" and "integer happy" as programmers; lots of things are represented as strings and integers which could have more information tracked in the type system; we don't want to be assigning customer names to customer addresses. A while back I wrote a series of blog posts (never finished!) about writing a virtual machine in OCaml, and one of the best things I did was wrapped every integer in the virtual machine with a type that indicates its purpose. That prevented so many bugs! OCaml makes it very easy to create little wrapper types; C# does not.

Second, I would not worry too much about duplicating the code. It's mostly an easy copy-paste, and you are unlikely to edit the code much or make mistakes. Spend your time solving real problems. A little copy-pasted code is not a big deal.

If you do want to avoid the copy-pasted code, then I would suggest using generics like this:

struct App {}
struct Payment {}

public struct Id<T>
{
    private readonly Guid _value;
    public Id(string value)
    {            
        var val = Guid.Parse(value);
        CheckValue(val);
        _value = val;
    }

    public Id(Guid value)
    {
        CheckValue(value);
        _value = value;           
    }

    private static void CheckValue(Guid value)
    {
        if(value == Guid.Empty)
            throw new ArgumentException("Guid value cannot be empty", nameof(value));
    }

    public override string ToString()
    {
        return _value.ToString();
    }
}

And now you're done. You have types Id<App> and Id<Payment> instead of AppId and PaymentId, but you still cannot assign an Id<App> to Id<Payment> or Guid.

Also, if you like using AppId and PaymentId then at the top of your file you can say

using AppId = MyNamespace.Whatever.Id<MyNamespace.Whatever.App>

and so on.

Third, you will probably need a few more features in your type; I assume this is not done yet. For example, you'll probably need equality, so that you can check to see if two ids are the same.

Fourth, be aware that default(Id<App>) still gives you an "empty guid" identifier, so your attempt to prevent that does not actually work; it will still be possible to create one. There is not really a good way around that.

Complicacy

"There is not really a good way around that." Is there a bad way? – Placable

@BurnsBA: You could try to educate your developers not to do that. A static analysis tool (e.g., coding contracts) could probably be configured to detect this sort of error. You could use a class instead of a struct. There are plenty of bad ways. Anyhow, keep in mind that avoiding default is also an issue for normal Guids (in fact, it's an issue for any struct). – Jyoti

@Placable You can check the state of the structure in every method and throw an exception if the guid is empty. This way, you will probably not be informed immediately when it is created, but eventually yes. – Wesleywesleyan

Do you think it would be useful to have a conversion to Guid defined for the type, or to have a public read-only property exposing the Guid? This would be useful for interfacing persistence frameworks, where the “raw” Guid is required. – Chiro

@dasblinkenlight: That seems like a reasonable choice. Maybe an explicit conversion to and from Guid would be useful. – Complicacy

A small interesting piece of information: A type that is used in the way that T is in this code is called a phantom type in the context of functional programming: It is used as a type parameter, but there is never a value of type T. – Lillianalillie

@Lii: I did not know that! That's funny, because when we were designing dynamic for C# 4, for a while we had a design that involved inferring a type we called the "phantom type", but it meant something completely different. – Complicacy

Could you also have Id be a non-generic class and use inheritance? – Sverdlovsk

@SolomonUcko: I think that is an even better design in most cases. That is, to have a specific class AppId extend Id. Or to combine the two approaches and have AppId extend Id<AppId>. – Lillianalillie

@Lii: Though that is a reasonable idea, the problem with it is that you only get extension with reference types, and that then causes collection pressure and excess memory allocation. Read the last paragraph of the original poster's question; they explicitly say they want a struct-based solution for these reasons. – Complicacy

@EricLippert: If you are using ImmutableObject on-label, I don't think it's really appropriate here; there are no public subproperties (and even if it's appropriate, it's a distraction). If you're using the ImmutableObject to indicate that Id<T> is immutable, then that is an off-label use of ImmutableObject (in fact, that attribute is often applied to mutable objects). See discussion at https://mcmap.net/q/390915/-how-is-immutableobjectattribute-used/18192 . That particular attribute is poorly named. – Jyoti

@Brian: Thanks for the note; I merely copy-pasted the code out of the original poster's question, and assumed that they knew what they were doing. You are right that likely they did not. – Complicacy

Does this technique (wrapping primitive type because of purpose) has some official name? – Monkeypot

I got really excited about this, then went to implemented it and realized I needed to inherit from interfaces that all had the ID, the inheritors needed a different <T> so had to go back to the drawing board. – Oquassa

Answer 2

We do the same, it works great.

Yes, it's a lot of copy and paste, but that is exactly what code-generation is for.

In Visual Studio, you can use T4 templates for this. You basically write your class once and then have a template where you say "I want this class for App, Payment, Account,..." and Visual Studio will generate you one source code file for each.

That way you have one single source (The T4 template) where you can make changes if you find a bug in your classes and it will propagate to all your Identifiers without you having to think about changing all of them.

Tyrannize

Answer 3

This is fairly easy to implement using record.

public readonly record struct UserId(Guid Id)
{
    public override string ToString() => Id.ToString();
    public static implicit operator Guid(UserId userId) => userId.Id;
}

The implicit operator allows us to use the strongly typed UserId as a regular Guid where applicable.

var id = Guid.NewGuid();
GuidTypeImportant(id);                 // ERROR
GuidTypeImportant(new UserId(id));     // OK
DontCareAboutGuidType(new UserId(id)); // OK
DontCareAboutGuidType(id);             // OK

void GuidTypeImportant(UserId id) { }
void DontCareAboutGuidType(Guid id) { }

Tompion

This is surely the correct answer, since the advent of C# 10. – Immiscible

Answer 4

This has a nice side effect. You can have these overloads for an add:

void Add(Account account);
void Add(Payment payment);

However you cannot have overloads for the get:

Account Get(Guid id);
Payment Get(Guid id);

I always disliked this asymmetry. You have to do:

Account GetAccount(Guid id);
Payment GetPayment(Guid id);

With the above approach this is possible:

Account Get(Id<Account> id);
Payment Get(Id<Payment> id);

Symmetry achieved.

Manvel