Avoid circular reference for classes

Question

I have a function that creates and returns a "Person". The "Person" has a "Spouse" property which is, of course, another "Person". This causes an endless recursion since the "Person" being created is always a "new" one each time the function is called. Is there a way to use the same function (as shown below) without causing an endless loop?

public PersonModel Load(int personID)
{
    PersonModel person = new PersonModel();
    using (SqlConnection conn = new SqlConnection())
    {
        conn.ConnectionString = Helpers.ConnectionDB;
        SqlCommand command = new SqlCommand();
        command.Connection = conn;
        command.CommandType = CommandType.StoredProcedure;
        command.CommandText = "LoadPerson";
        command.Parameters.Add(new SqlParameter("@PersonID", personID));
        conn.Open();
        SqlDataReader reader = command.ExecuteReader();
        if (reader.Read())
        {
            person.PersonID = int.Parse(reader["PersonID"].ToString());
            person.FirstName = reader["FirstName"].ToString();
            person.LastName = reader["LastName"].ToString();
            person.MiddleName = reader["MiddleName"].ToString();
            person.Age = reader["Age"] != DBNull.Value ? int.Parse(reader["Age"].ToString()) : (int?)null;
            person.SpouseID = reader["SpouseID"] != DBNull.Value ? int.Parse(reader["SpouseID"].ToString()) : (int?)null;
            if (person.SpouseID != null && person.Spouse == null)
            {
                person.Spouse = this.Load(person.SpouseID.Value);
            }
        }
        conn.Close();
    }
    return person;
}

You might be better-off writing a recursive CTE to load the social-graph instead of doing it in application-code, it will be less network-chatty that way and only require a single command execution. — Dai, Oct 17 '16 at 23:39
In a case like this where A references B and B references A, I would only only have hold the SpouseID, and not a Person object. If you want to get details of the Spouse then you create a Spouse from the ID. Otherwise you hit problems when you try to serialize this sort of relationship. — DeanOC, Oct 17 '16 at 23:42
Dai -- I'm not familiar with what you're referring to. Are you suggesting that I load the spouse (if there is one) within the sproc itself and then create that object from the secondary table I return? I was hoping to avoid that since I was going to have a similar function that returns ALL "Persons" (as opposed to just a specific one). — Starfleet Security, Oct 17 '16 at 23:44
Add a 2nd parameter to parameter list bool spouse. Only call the function a 2nd time when spouse is false. — jdweng, Oct 18 '16 at 00:02

score 2 · Accepted Answer · answered Oct 17 '16 at 23:42

Use a Dictionary<Int32,PersonModel> to keep track of loaded entities:

public PersonModel Load(Dictionary<Int32,PersonModel> dict, int personId) {

    PersonModel ret;
    if( dict.TryGetValue( personId, out ret ) ) return ret;

    // load from database here, but do not call recursively just yet
    ret = new PersonModel() { ... };

    dict.Add( ret.PersonId, ret );

    ret.Spouse = this.Load( dict, person.SpouseId.Value );
}

score 0 · Answer 2 · answered Oct 18 '16 at 00:00

I can think of a couple different options:

Add an optional parameter to Load() to indicate that you should not attempt to load the spouse. This is a bit of a hack but very simple.

public PersonModel Load(int personID, bool loadSpouse = true)
{
    ...
    if (loadSpouse && person.SpouseID != null && person.Spouse == null)
    {
        person.Spouse = this.Load(person.SpouseID.Value, false);
    }
    ...

    return person;
}

Select the spouse's properties in the stored proc by using a left outer join. This would perform better and if you ever wanted to select all employees this would be much more efficient since it would be just one database access. You would add code like this in the if statement to build the spouse object:
```
person.Spouse = new PersonModel()
person.Spouse.FirstName = reader["SpouseFirstName"].ToString();
...
```

Shimmy Weitzhandler · Answer 3 · 2017-06-05T06:35:09.037

Not exactly per the OPs question, but since I landed here because of the title, it might be beneficial adding a general answer to this.

So in my scenario I needed to change a property in an entire graph setting all its sub properties to any depth.

In order to ensure I don't get an instance performed twice (thus allowing StackOverflowException), I'm passing a HashSet<T> along each call, so we know to skip the performed items:

//The executed parameter needn't ever be provided externally.
static void SetState(IObject obj, State state, HashSet<IObject> executed = null)
{
  if(executed == null) executed = new HashSet<IObject>(/* provide comparer if needed*/);
  if(!executed.Add(obj)) return;

  //safe to continue walking graph
  obj.State = state;
  var props = obj.GetObjectPropertyValues().OfType<IObject>();
  foreach (var prop in props)
    SetState(obj, state, executed);
}

Note, the items must implement proper equality and GetHashCode functions, or a comparer argument should be passed to the HashSet's constructor.

Avoid circular reference for classes

3 Answers3