2
\$\begingroup\$

I am converting a VB.NET app over to C#. I won't subject you to the old code but the following is done in a little over 100 lines using nested loops. I am using Entity Framework 6 as my ORM. I have cleaned that up some to this:

public void Remove(int key)
{
    var myObject = _context.MyObjects.FirstOrDefault(x => x.QueryKey == key);
    var myObjectInputs = _context.MyObjectInputs.Where(x => x.QueryKey == key);
    var myObjectInputProperties = _context.MyObjectInputProperties.Where(x => myObjectInputs.Select(y => y.WQI_Key).Contains(x.WQI_Key));
    var myObjectOutputs = _context.MyObjectOutPuts.Where(x => x.QueryKey == key);
    var myObjectOutputProperties = _context.MyObjectOutputProperties.Where(x => myObjectOutputs.Select(y => y.WQO_Key).Contains(x.WQO_Key));
    var myObjectGroups = _context.MyObjectGroups.Where(x => x.QueryKey == key);

    _context.MyObjects.Remove(myObject);
    _context.MyObjectInputs.RemoveRange(myObjectInputs);
    _context.MyObjectInputProperties.RemoveRange(myObjectInputProperties);
    _context.MyObjectOutPuts.RemoveRange(myObjectOutputs);
    _context.MyObjectOutputProperties.RemoveRange(myObjectOutputProperties);
    _context.MyObjectGroups.RemoveRange(myObjectGroups);
    _context.SaveChanges();
}

I still don't like it. The method is huge and doesn't follow the single responsibility principle. I considered refactoring to this:

public void Remove(int key)
{
    var myObject = _context.MyObjects.FirstOrDefault(x => x.QueryKey == key);
    _context.MyObjects.Remove(myObject);
    RemoveChildren(key);
    _context.SaveChanges();
}

private void RemoveChildren(int key)
{
    RemoveInputs(key);
    RemoveOutputs(key);
    RemoveGroups(key);
}

private void RemoveGroups(int key)
{
    var myObjectGroups = _context.MyObjectGroups.Where(x => x.QueryKey == key);
    _context.MyObjectGroups.RemoveRange(myObjectGroups);
}

private void RemoveOutputs(int key)
{
    var myObjectOutputs = _context.MyObjectOutPuts.Where(x => x.QueryKey == key);
    var myObjectOutputProperties =
        _context.MyObjectOutputProperties.Where(x => myObjectOutputs.Select(y => y.WQO_Key).Contains(x.WQO_Key));
    _context.MyObjectOutPuts.RemoveRange(myObjectOutputs);
    _context.MyObjectOutputProperties.RemoveRange(myObjectOutputProperties);
}

private void RemoveInputs(int key)
{
    var myObjectInputs = _context.MyObjectInputs.Where(x => x.QueryKey == key);
    var myObjectInputProperties =
        _context.MyObjectInputProperties.Where(x => myObjectInputs.Select(y => y.WQI_Key).Contains(x.WQI_Key));
    _context.MyObjectInputs.RemoveRange(myObjectInputs);
    _context.MyObjectInputProperties.RemoveRange(myObjectInputProperties);
}

This seems like an okay option, but I am wondering if there is a way to remove all of these items in one fell swoop.

Obviously in a perfect world I would associate these objects in the database and have them cascade delete; however, I cannot edit our database structure at this time.

\$\endgroup\$
2
\$\begingroup\$

Can this method be changed to remove all objects in one call?

There is no way to remove multiple objects from multiple tables in one statement and there shouldn't be (cascading foreign key constraints notwithstanding).

The problem is that Remove(int key) is a poor name for that function. Something like RemoveObjectInputsOutputsPropertiesAndGroups(int key) would be better. Similarly, I would have on idea what a RemoveChildren() function should do in this context.

\$\endgroup\$
2
  • \$\begingroup\$ typo: "would be better better" \$\endgroup\$ – ANeves thinks SE is evil Jul 28 '14 at 15:57
  • \$\begingroup\$ The methods in my real code are named better. I posted highly sanitized code here just to get a clear picture on what to do about the remove and removeRange statements \$\endgroup\$ – Robert Jul 28 '14 at 15:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.