0
\$\begingroup\$

I have following code which gets MastrSegments from repository(entity framework). It's working but I think I am violating DRY (though not sure).

MastrSegments conatins collection of SegReqs and SegReq contains collection Routes. And I am looping through them to update RootObject

    foreach (var MastrSegment in MastrSegments)
    {

        var segReqCount = MastrSegment.segReqs.Count();

        var MastrSegmentRow = new Row
        {
            c = new RootObject[] { new RootObject { v = MastrSegment.Name }, new RootObject { v = "TopClient" }, new RootObject { v = segReqCount }, new RootObject { v = 12 } }

        };

        foreach (var segReq in MastrSegment.segReqs)
        {

            var routesCount = segReq.routess.Count;
            Row segReqRow;

            if (segReq.Name != MastrSegment.Name)
            {
                segReqRow = new Row
                {
                    c = new RootObject[] { new RootObject { v = segReq.Name }, new RootObject { v = MastrSegment.Name }, new RootObject { v = routesCount }, new RootObject { v = -26 } }

                };
            }
            else
            {
                throw new NotSupportedException("MastrSegment name and  segReq name can not be same !!");
            }


            foreach (var routes in segReq.routess)
            {
                var circuitsCount = routes.Circuits.Count;
                Row routesRow = null;

                if (routes.Name != segReq.Name)
                {
                     routesRow = new Row
                    {
                        c = new RootObject[] { new RootObject { v = routes.Name }, new RootObject { v = segReq.Name }, new RootObject { v = circuitsCount }, new RootObject { v = 16 } }

                    };
                }
                else
                {
                    throw new NotSupportedException("segReq name and  routes name can not be same !!");
                }

                rows.Add(routesRow);
            }

            rows.Add(segReqRow);
        }


        rows.Add(MastrSegmentRow);
    }

    var returnData = rows.ToArray();
\$\endgroup\$
3
\$\begingroup\$

when ever you want to solve violations of the DRY principle you have to convert similar code into equal code.

If you look at the two inner loops: make the most inner loop using new local variables to replace the differeing parts

       {
            var circuitsCount = routes.Circuits.Count;
            Row routesRow = null;

            if (routes.Name != segReq.Name)
            {
                 routesRow = new Row
                {
                    c = new RootObject[] { new RootObject { v = routes.Name }, new RootObject { v = segReq.Name }, new RootObject { v = circuitsCount }, new RootObject { v = 16 } }

                };
            }
            else
            {
                throw new NotSupportedException("segReq name and  routes name can not be same !!");
            }

            rows.Add(routesRow);
        }

becomes

       { // may be no valid syntax since I'm not a C# guy...
            var count = routes.Circuits.Count;
            Row routesRow = null;
            var master = "routes"
            var masterName = routes.Name;
            var detail = "segReq"
            var detailName = segReq.Name;
            var magicNumber =  16;

            if (masterName != detailName )
            {
                 routesRow = new Row
                {
                    c = new RootObject[] { new RootObject { v = masterName }, new RootObject { v = detailName }, new RootObject { v = count }, new RootObject { v =  magicNumber } }

                };
            }
            else
            {
                throw new NotSupportedException(detail+" name and "+master+" name can not be same !!");
            }

            rows.Add(routesRow);
        }

now you extract the "future common" part to a new method:

       {
            var count = routes.Circuits.Count;
            var master = "routes"
            var masterName = routes.Name;
            var detail = "segReq"
            var detailName = segReq.Name;
            var = magicNumber  16;

            Row routesRow = CreateMasterDetailRow(master, masterName, detail, detailName, count, magicNumber);
            rows.Add(routesRow);
        }
//...

func Row CreateMasterDetailRow(var master, var masterName, var detail, var detailName, var count, var magicNumber)
{

            if (masterName != detailName )
            {
                 return new Row
                {
                    c = new RootObject[] { new RootObject { v = masterName }, new RootObject { v = detailName }, new RootObject { v = circuitsCount }, new RootObject { v =  magicNumber } }

                };
            }
            else
            {
                throw new NotSupportedException(detail+" name and "+master+" name can not be same !!");
            } 
}

inline the extracted variables into the function call:

       {
            Row routesRow = CreateMasterDetailRow("routes", routes.Name, "segReq", segReq.Name, routes.Circuits.Count, 16);
            rows.Add(routesRow);
        }

then use the call in the middle loop:

   {  // maybe I got the order of "master" and "detail" wrong...
        Row segReqRow= CreateMasterDetailRow("MastrSegment ", MastrSegment.Name, "segReq", segReq.Name, segReq.routess.Count, -26);
        foreach (var routes in segReq.routess)
        {
            Row routesRow = CreateMasterDetailRow("routes", routes.Name, "segReq", segReq.Name, routes.Circuits.Count, 16);
            rows.Add(routesRow);
        }
        rows.Add(segReqRow);

You can even replace the similar code in the outer loop if you provide some "fake data" for the (not failing) consistency check to satify the new methods parameter list...

\$\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.