Entity Framework with multiple connections open?

Question

I'm learning Entity Framework and I'm wondering if this code is the proper way of doing this. Basically this is my attempt at refactoring a very long controller in ASP.Net MVC 4. I'm using Entity Framework 5 as well.

What I did was I separated out some code into a more common method (buildUpdatedAnswerDetailRecord).

Notice in my parent function I have a connection open because I'm doing other DB related operations (population of data into properties). But then in the buildUpdatedAnswerDetailRecord method I'm opening another connection and then passing back a AnswerDetail.

Is this the right way to do this or should I have just kept 1 connection open and just duplicated code?

using (var db = new LEAP_Professional_DAL.DAL.LEAPEntitiesDAL())
{

//local variable of a AnswerDetail    
AnswerDetail adItem;

//Doing a lot of other Database work (filling other properties)
....

adItem = buildUpdatedAnswerDetailRecord(evaluation.IsLeader, item, backButtonItem.AnswerDetailKey);
db.Entry(adItem).State = EntityState.Modified;

....
//Doing a lot of other Database work (filling other properties)


}

    private AnswerDetail buildUpdatedAnswerDetailRecord(Boolean isLeader, EvaluationObject item, int? detailKey)
    {
        try
        {
            AnswerDetail buildAdItem;
            using (var db = new DAL.DAL.EntitiesDAL())
            {
                buildAdItem = db.AnswerDetails.Find(detailKey);
                if (buildAdItem != null)
                {
                    ((IObjectContextAdapter)db).ObjectContext.Detach(buildAdItem);
                    if (isLeader == true)
                    {
                        buildAdItem.Comment = item.LeaderComment;
                        buildAdItem.AnswerOptionKey = item.LeaderAnswerOptionKey.Value;
                        buildAdItem.UpdatedBy = userName;
                        buildAdItem.UpdateStamp = DateTime.Now;
                    }
                    else
                    {
                        buildAdItem.Comment = item.TeacherComment;
                        buildAdItem.AnswerOptionKey = item.TeacherAnswerOptionKey.Value;
                        buildAdItem.UpdatedBy = userName;
                        buildAdItem.UpdateStamp = DateTime.Now;
                    }


                }
            }
            return buildAdItem;
        }
        catch (Exception ex)
        {
            throw new Exception("buildUpdatedAnswerDetailRecord Error Occured: " + ex.Message.ToString());
        }
    }

Answer 1

This answer is just a little remark about this redundant part:

if (isLeader == true)
{
    buildAdItem.Comment = item.LeaderComment;
    buildAdItem.AnswerOptionKey = item.LeaderAnswerOptionKey.Value;
    buildAdItem.UpdatedBy = userName;
    buildAdItem.UpdateStamp = DateTime.Now;
}
else
{
    buildAdItem.Comment = item.TeacherComment;
    buildAdItem.AnswerOptionKey = item.TeacherAnswerOptionKey.Value;
    buildAdItem.UpdatedBy = userName;
    buildAdItem.UpdateStamp = DateTime.Now;
}

Which could be simplified to:

buildAdItem.UpdatedBy = userName;
buildAdItem.UpdateStamp = DateTime.Now;

if (isLeader)
{
    buildAdItem.AnswerOptionKey = item.LeaderAnswerOptionKey.Value;
    buildAdItem.Comment = item.LeaderComment;
}
else
{
    buildAdItem.AnswerOptionKey = item.TeacherAnswerOptionKey.Value;
    buildAdItem.Comment = item.TeacherComment;
}

---

• Don't compare a bool to true or false - just use the Boolean value as the condition, that's what a bool is for!
• Avoid copy+pasting code. There's never a very good reason to do that.

---

Is this the right way to do this or should I have just kept 1 connection open and just duplicated code?

Your code structure isn't very clear (it's actually very unclear) from what you've given us - based on what I'm seeing I don't see why the two methods couldn't be using the same DbContext, nor why using the same connection would result in "duplicated code".

Also the entity state manager is pretty good at managing entities all by itself, it seems to me that you're making your life harder than it needs to be by detaching entities and managing state yourself.

Answer 2

Just a quick note about the way exceptions are handled:

catch (Exception ex)
{
    throw new Exception("buildUpdatedAnswerDetailRecord Error Occured: " + ex.Message.ToString());
}

You're shooting yourself in the foot: you're receiving a potentially very meaningful and helpful exception, with a very detailed stack trace, then you're grabbing its message and throwing everything else away (including the stack trace), and throwing a meaningless System.Exception.

At the very least, you should be putting ex as the new exception's InnerException, so as to retain all the details about the exception that was thrown. See this answer for more details about why throwing System.Exception is bad.

Answer 3

yes, you can pass db context through functions

using(MyEntities context = new MyEntities())
{
  SomeObject record = //pull record;

  SomeFunction(record, context);

}//connection closes here

void SomeFunction(SomeObject record, MyEntities context)
{
  //do some stuff on record
}