Álvaro García
Álvaro García

Reputation: 19356

C# refactoring considerations

I have the following doubt.

For refactoring, I have read that is good create methods that has a very specific responsability, so if it is possible, it is a good idea to split a complex method in others small methods.

But imagine that I have this case:

I have to create a list of objects, and insdie this objects, I have to create another object. Something like that:

public void myComplexMethod(List<MyTypeA> paramObjectsA)
{
    foreach(MyTypeA iteratorA in paramObjectsA)
    {
        //Create myObjectB of type B

        //Create myObjectC of type C

        myObjectB.MyPorpertyTpyeC = myObjectC;
    }
}

I can split this method in two methods.

public void myMethodCreateB(List<MyTypeA> paramObjectsA)
{
    foreach(MyTypeA iteratorA in paramObjectsA)
    {
        //Create myObjectB of type B
    }
}



public void myMethodCreateB(List<MyTypeB> paramObjectsB)
{
    foreach(MyTypeB iteratorB in paramObjectsB)
    {
        //Create myObjectC of type C
        iteratorB.PropertyC = myObjectC;
    }
}

In the second option, when I use two methods instead one, the unit tests are less complex, but the problem is that I use two foreach loops, so it is less efficient than use only one loop like in the first option.

So, what is the best practice, at least in general, to use a method a bit more complex to be more efficient or to use more methods?

Thanks so much.

Upvotes: 0

Views: 83

Answers (2)

Ryan
Ryan

Reputation: 815

Usually I would keep using one for-loop in this case. Seems you are just create and decorate the objects of MyTypeB. I would prefer create a factory method in class MyTypeB:

static MyTypeB Create(MyTypeA a) { // if the creation of MyTypeB depends on A
    //Create myObjectB of type B
    //Create myObjectC of type C
    myObjectB.MyPorpertyTpyeC = myObjectC;
    return myObjectB;
}

then your complex method will become:

public void myComplexMethod(List<MyTypeA> paramObjectsA)
{
    foreach(MyTypeA iteratorA in paramObjectsA)
    {
        MyTypeB myObjectB = MyTypeB.Create(iteratorA);
    }
}

Upvotes: 2

Christoph
Christoph

Reputation: 2317

I generally put readability at a higher priority than performance, until proven otherwise. I'm generalizing now a bit but in my experience, when people focus on performance too much at the code level, the result is less maintainable code, it distracts them from creating functionally correct code, it takes longer (=more money), and possibly results in even less performant code.

So don't worry about it and use the more readable approach. If your app is really too slow in the end, run it through a profiler and pinpoint (and prove) the one or two places where it requires optimization. I can guarantee you it won't be this code.

Making the correct choices at the architectural level early on is much more critical because you won't be able to easily make changes at that level once your app is built.

Upvotes: 3

Related Questions