The general relationship you have between your classes (0..1 User to 0..1 Thing) is a perfectly valid kind of relationship. It's less common, but valid in the right situation, which this seems to be.
However, you need to be very careful here. Your current setup creates a lot of room for contradiction. Part of this is because you store this information twice. Another part of the issue is that you are treating User
and Thing
as equals, where either of them is allowed to direct the other. That's causing you a lot of grief because there's no clear hierarchy.
These issues are closely tied together. If you stop treating them as equals and instead assign one of them to be the owner of the other; then you can resolve a lot of this ambiguity and keep your code more elegant.
When you store the relationship twice, you start running the risk of things contradicting each other. Using a simpler (somewhat silly) example:
public class Foo
{
public string Name { get; set; }
public int LengthOfName { get; set; }
}
These two pieces of information stem from the same source. But because we store them separately, we're able to create states which contradict one another:
var foo = new Foo() { Name = "Bob", LengthOfName = 5 };
You might say that this is clearly wrong and easily fixed, but it can get very messy:
var foo = new Foo() { Name = "Bob", LengthOfName = 3 };
// lots of logic
foo.Name = "Robert";
Console.WriteLine(foo.LengthOfName); // Hey, why is this 3!?
What we should do here is ensure that these two pieces of information stem from the same root:
public class Foo
{
public string Name { get; set; }
public int LengthOfName => Name.Length;
}
Now, it's impossible for the name and its length to go out of sync.
This same issue applies to your case:
var user = new User();
var differentUser = new User();
var thing = new Thing();
var differentThing = new Thing();
user.Thing = thing;
thing.User = differentUser; // That's not symmetrical!
differentUser.Thing = differentThing; // And neither is this!
The fact that this is possible means that your logic is allowing for more options than are allowed by your business logic.
The simple solution is to only store one of these references. You get to pick which one you want (this related to the second issue I pointed out, i.e. a lack of hierarchy), but the end result is that one of them holds a reference to the other, but not vice versa.
This means that there's no longer a contradiction. However, this also limits your navigation properties, as you can now only travel from one to the other, not the other way around.
If that's not a problem for you, great! Stop reading here and enjoy your solution. But if you do want two-way navigation, read on.
When you are dealing with an ORM (I'll use Entity Framework here as an example), then you can work around this issue by defining two navigational properties but only one FK:
public class User
{
public virtual Thing Thing { get; set; }
}
public class Thing
{
public int UserId { get; set; }
public virtual User User { get; set; }
}
However, if you're dealing with an in-memory collection or a data store that doesn't allow you to separate the FK from the navigational property, then your options are limited.
Either you have the dual relationship (which is an open door to bugs), a one-way navigation, or you have to develop a way for the other direction of the relationship to be resolved, e.g.:
public class User
{
// User can directly navigate to Thing
public Thing Thing { get; set; }
}
public class Thing
{
// Thing has to indirectly go out and find its User
public User User
{
get
{
var allUsers = ....; // get a list of all users
return allUsers.SingleOrDefault(user => user.Thing == this);
}
}
}
But it's not hard to see that this requires access to your complete datastore. Admittedly, when it's in-memory, that's not a huge cost, but it is a dependency that shouldn't really be there (I think this is debatable either way - whether the dependency is justified by needing to use it or not)
Similarly, you can also delegate the "find a thing's user" logic to a repository method and just skip the navigational property altogether. Or you can set your repository up so that you always return a user and their thing together, and you never have to fetch one based on the other.
You could also change the inferior object (if you've decided a hierarchy) to be a value object that is part of the other object, but then these objects cease to have their own lifetime. It's unclear whether this is suitable for your case.
As you can see, there are many ways to resolve this issue (and I don't think I've exhausted the options yet), but you need to figure out which one is the right one for your case.
Thing
but notUser
?