1
\$\begingroup\$

What would be the most elegant way to refactor the following method?

 public void AddOrUpdateDesignCVT(DesignInformation designInformation,
 CommonTransformerProperties commonTransformerProperties,
 SecondaryWindings secondaryWindings,
 CapacitorVoltageDivider capacitorVoltageDivider,
 IntermediateVoltageTransformer intermediateVoltageTransformer,
 SpecialDesignInformation specialDesignInformation,
 TransformerLimits transformerLimits)
 {
 GenerateWindingInstructions(designInformation);
 Design design;
 using (var context = new DesignContext())
 {
 if (!DesignExist(designInformation.DesignID))
 {
 context.DesignInformation.Add(designInformation);
 context.CommonTransformerPropeteries.Add(commonTransformerProperties);
 context.SecondaryWindings.Add(secondaryWindings);
 context.CapacitorVoltageDivider.Add(capacitorVoltageDivider);
 context.IntermediateVoltageTransformer.Add(intermediateVoltageTransformer);
 context.SpecialDesignInformation.Add(specialDesignInformation);
 context.TransformerLimits.Add(transformerLimits);
 context.SaveChanges();
 return;
 }
 }
 using (var newcontext = new DesignContext())
 {
 design = GetDesingByNameFromDatabase(newcontext, designInformation.DesignID, designInformation.Technology);
 UpdateSecondaryWindings(design, newcontext, designInformation, secondaryWindings);
 UpdateSpecialWires(design, newcontext, designInformation, specialDesignInformation);
 newcontext.SaveChanges();
 }
 int dbId = design.DesignInformation.DataBaseID;
 design.DesignInformation = designInformation;
 design.DesignInformation.DataBaseID = dbId;
 design.CommonTransformerProperties = commonTransformerProperties;
 design.CommonTransformerProperties.DataBaseID = dbId;
 design.SecondaryWindings = secondaryWindings;
 design.SecondaryWindings.DataBaseID = dbId;
 design.CapacitorVoltageDivider = capacitorVoltageDivider;
 design.CapacitorVoltageDivider.DataBaseID = dbId;
 design.IntermediateVoltageTransformer = intermediateVoltageTransformer;
 design.IntermediateVoltageTransformer.DataBaseID = dbId;
 design.SpecialDesignInformation = specialDesignInformation;
 design.SpecialDesignInformation.DataBaseID = dbId;
 using (var newcontext = new DesignContext())
 {
 newcontext.Entry(design.CommonTransformerProperties).State = System.Data.Entity.EntityState.Modified;
 newcontext.Entry(design.SecondaryWindings).State = System.Data.Entity.EntityState.Modified;
 newcontext.Entry(design.CapacitorVoltageDivider).State = System.Data.Entity.EntityState.Modified;
 newcontext.Entry(design.IntermediateVoltageTransformer).State = System.Data.Entity.EntityState.Modified;
 newcontext.Entry(design.SpecialDesignInformation).State = System.Data.Entity.EntityState.Modified;
 newcontext.Entry(design.DesignInformation).State = System.Data.Entity.EntityState.Modified;
 newcontext.SaveChanges();
 return;
 }
 }
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Feb 12, 2016 at 15:57
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

1) Possibly better structure

Since information from DesignInformation, CommonTransformerProperties, ..., TransformerLimits objects seem to be semantically linked, it should be grouped into a single object type (maybe a Transformer). The EF model should be similar to the class below (greatly simplified).

class Transformer
{
 int TransformerId { get; set; }
 DesignInformation designInformation;
 CommonTransformerProperties commonTransformerProperties,
 SecondaryWindings secondaryWindings,
 CapacitorVoltageDivider capacitorVoltageDivider,
 IntermediateVoltageTransformer intermediateVoltageTransformer,
 SpecialDesignInformation specialDesignInformation,
 TransformerLimits transformerLimits
}

The real gain here is that you can have a single ID to be generated, but you have to change child types to link them to the parent. I.e. DesignInformation, CommonTransformerProperties, ..., TransformerLimits should also have a TransformerId specified. Persistence code should be simpler:

 using (var newcontext = new DesignContext())
 {
 design = GetDesingByNameFromDatabase(newcontext, designInformation.DesignID, designInformation.Technology);
 UpdateSecondaryWindings(design, newcontext, designInformation, secondaryWindings);
 UpdateSpecialWires(design, newcontext, designInformation, specialDesignInformation);
 newcontext.SaveChanges();
 }
 int dbId = design.DesignInformation.DataBaseID;
 design.DesignInformation = designInformation;
 design.DesignInformation.DataBaseID = dbId;
 design.CommonTransformerProperties = commonTransformerProperties;
 design.CommonTransformerProperties.DataBaseID = dbId;
 design.SecondaryWindings = secondaryWindings;
 design.SecondaryWindings.DataBaseID = dbId;
 design.CapacitorVoltageDivider = capacitorVoltageDivider;
 design.CapacitorVoltageDivider.DataBaseID = dbId;
 design.IntermediateVoltageTransformer = intermediateVoltageTransformer;
 design.IntermediateVoltageTransformer.DataBaseID = dbId;
 design.SpecialDesignInformation = specialDesignInformation;
 design.SpecialDesignInformation.DataBaseID = dbId;

should be:

 using (var newcontext = new DesignContext())
 {
 var newTransformer = new Transformer();
 newcontext.Transformer.Add(newTransformer);
 design = GetDesingByNameFromDatabase(newcontext, designInformation.DesignID, designInformation.Technology);
 UpdateSecondaryWindings(design, newcontext, designInformation, secondaryWindings);
 UpdateSpecialWires(design, newcontext, designInformation, specialDesignInformation);
 newTransformer.DesignInformation = designInformation;
 newTransformer.CommonTransformerProperties = commonTransformerProperties;
 // ...
 newcontext.SaveChanges();
 }

EF will automatically push generated identifier to all children entities.

2) Database context usage

Database context should not be disposed so often. Usually Unit of work pattern is used to simplify its usage and also group everything that must be saved together (i.e. in a transaction). More details can be found here and here.

This will spare you from passing the context for calls like:

 design = GetDesingByNameFromDatabase(newcontext, designInformation.DesignID, designInformation.Technology);
 UpdateSecondaryWindings(design, newcontext, designInformation, secondaryWindings);
 UpdateSpecialWires(design, newcontext, designInformation, specialDesignInformation);

Those functions can use the context by just calling DesignContext.Instance method where the method would look like this:

 public static Instance
 {
 get
 {
 // Dirty (non thread-safe) check
 if (UnitOfWorkStore.GetData(DATACONTEXT_KEY) == null)
 {
 lock (objSync)
 {
 // Thread-safe check
 if (UnitOfWorkStore.GetData(DATACONTEXT_KEY) == null)
 {
 var context = new DesignContext();
 UnitOfWorkStore.SetData(DATACONTEXT_KEY, context);
 }
 }
 }
 return (DesignContext)UnitOfWorkStore.GetData(DATACONTEXT_KEY);
 }
 }

An improvement is to using Dependency Injection and to have the context injected. The whole process is explained in great detail here. Of course, other DI frameworks - such as Ninject - can be used.

answered Feb 12, 2016 at 21:33
\$\endgroup\$

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.