0
\$\begingroup\$

I have an application that allows a user to create a poll that includes questions and answer choices as part of the poll. The hierarchy is a poll has one or more questions, a question has one or more answers, and each answer has a vote.

The following code is my implementation that allows the updating of an existing poll. The update allows for properties of the poll to be updated, including the addition or removal of children.

The implementation works, but it feels like there is a better way to achieve the functionality. As you will see from my code, I'm creating a context object each time I want to add or update any of the objects.

 public string Update(Poll poll)
 {
 try
 {
 if (poll == null)
 return null;
 using (var context = new PollContext())
 {
 context.Entry(poll).State = poll.PollId == Guid.Empty ? EntityState.Added : EntityState.Modified;
 context.Polls.AddOrUpdate(poll);
 context.SaveChanges();
 }
 //update the questions
 using (var context = new PollContext())
 {
 foreach (var question in poll.Questions)
 {
 context.Entry(question).State =
 question.QuestionId == Guid.Empty ? EntityState.Added : EntityState.Modified;
 context.Questions.AddOrUpdate(question);
 }
 context.SaveChanges();
 }
 //update the answers
 using (var context = new PollContext())
 {
 foreach (var answers in poll.Questions.Select(question => question.Answers).ToList())
 {
 var questionId = answers.ElementAt(0).QuestionId;
 var originalAnswerList = GetOriginalAnswerList(questionId);
 using (var context2 = new PollContext())
 {
 foreach (var originalAnswer in originalAnswerList)
 {
 if (answers.All(a => a.AnswerId != originalAnswer.AnswerId))
 {
 Delete(originalAnswer);
 Delete(originalAnswer.Votes);
 }
 }
 }
 foreach (var answer in answers)
 {
 context.Entry(answer).State =
 answer.AnswerId == Guid.Empty ? EntityState.Added : EntityState.Modified;
 context.Answers.AddOrUpdate(answer);
 context.Votes.AddOrUpdate(answer.Votes);
 }
 }
 context.SaveChanges();
 }
 return "Poll updated";
 }
 catch (NullReferenceException ex)
 {
 throw new NullReferenceException("A null exception occurred in updating poll.\r\n" + ex.InnerException);
 }
 catch (Exception ex)
 {
 throw new Exception("An exception occurred in the updating poll.\r\n" + ex.InnerException);
 }
 }
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Mar 10, 2016 at 17:00
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$
  1. You don't have to use a using statement. You can use an IoC container such as Autofac and let it dispose the PollContext for you.

  2. I think you probably should update the children collection in memory first and call context.SaveChange() only once at the end, so the whole update operations are wrapped with in a unit of a work.

  3. I don't think try/catch is needed here as you only re-throw the exception, which could be handled on a higher level.

  4. There's lots of repeating of:

    context.Entry(poll).State = poll.PollId == Guid.Empty ? EntityState.Added : EntityState.Modified;
    context.Polls.AddOrUpdate(poll);
    

    You should probably create a generic function for it.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
answered Mar 11, 2016 at 2:01
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$ Commented Mar 11, 2016 at 2:26
  • \$\begingroup\$ Cobyz, I tried doing the updates as a single unit of work, instead of each child object individually. That is where I'm having some trouble. I'm not able to get the children to update with the parent update. \$\endgroup\$ Commented Mar 11, 2016 at 13:52

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.