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;
            }
    }
\$\endgroup\$
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.

\$\endgroup\$

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.