Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Refactoring large method in .NET

If I have a large .NET method, I want to know if it is good practice to split it up into multiple methods or not. My concerns are: 1 - Is there any point creating a method if it will only be called once?
2 - If I'm calling a private method from within my class, I try not to use instance variables. Is this good or bad practice? For example:

var a, b;
public void TestMe
{
   FirstTest(a,b);
}
private void FirstTest(paramA, paramB)
{
         //code here with passed in parameters
}

//instead of:
private void FirstTest()
{
      //code here with instance variables a,b
}  

edit: replace global with instance

like image 370
MC. Avatar asked Mar 09 '10 20:03

MC.


People also ask

How do you refactor a method in C#?

Select Edit > Refactor > Extract Method. Right-click the code and select Refactor > Extract > Extract Method. Right-click the code, select the Quick Actions and Refactorings menu and select Extract Method from the Preview window popup.

What is refactoring a method?

Refactoring is the process of restructuring code, while not changing its original functionality. The goal of refactoring is to improve internal code by making many small changes without altering the code's external behavior.


2 Answers

With respect to your first question, reducing complexity in a method is sufficient reason to break a large method into small ones. Smaller methods are much easier to comprehend than a single large method. If done properly, by breaking the method up into logically consistent units of work, many small methods are preferrable to a single large method in almost all cases. The only conditions where it might not be is if it impacts your performance to the point where it is no longer acceptable from that perspective.

With respect to the second question, as @Jason points out, you are using instance variables not globals within your class. I would say which practice is preferred depends on the context of the method. Certainly if the method can be reused in many contexts, only some of which operate on instance variables, it ought to be parameterized. If you are only ever using the instance variables, you might choose not to have the parameters, and refactor as needed later for usability.

In C#, I would also prefer using instance properties over fields, to further decouple the property from the method using it.

like image 144
tvanfosson Avatar answered Oct 07 '22 04:10

tvanfosson


1 - Is there any point creating a method if it will only be called once?

Yes, there are many reasons to do this. Readability is perhaps the most important. If you can make a method more readable and maintainable by breaking it apart, then by all means do so.

In my experience with refactoring legacy code where a method is way too long, there are little pieces of code that appear over and over again. These are usually the best places to look for refactoring opportunities. Creating separate methods for those pieces can greatly decrease a method's length, and, thereby, greating increase its readability.

2 - If I'm calling a private method from within my class, I try not to use instance variables. Is this good or bad practice?

Usually, the smaller you can make a variable's scope the better. Like you, I tend to use parameters whenever possible. If a method has references only to its own parameters, then it becomes much easier to reason about the method, verify its correctness, and use it correctly. (And if a method can do this and not modify any state, then that buys you a lot of maintenance benefits.)

If the purpose of a method is best served by manipulating an object's fields, then that is perfectly acceptable, and in many cases, unaviodable. But, as you indicate, this is especially true with public methods. When refactoring a large method into smaller ones, I will rarely, if ever, access member fields directly in the new methods. This is mainly just to make it easer to reason about the program's behavior.

When refactoring in this manner, make sure you mark the new methods as static if they don't access any fields. This will make the intent explicit.

like image 24
Jeffrey L Whitledge Avatar answered Oct 07 '22 02:10

Jeffrey L Whitledge