dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.92k stars 4.02k forks source link

Split passed params into primitive types after ExtractMethod refactorings #7389

Open anjmao opened 8 years ago

anjmao commented 8 years ago

It is good practice to pass primitive type params for your methods until they need to be grouped into to class (as I remember 7 and more params). This is also called Law of Demeter.

Let's look at example

    class Employee
    {
        public int IdEmployee { get; set; }
        public string Name { get; set; }
        public bool IsActive { get; set; }
    }

    class EmployeeFormComponent
    {
        public IEnumerable<string> GetNames(IEnumerable<Employee> employees)
        {
            var result = new List<string>();
            foreach(var employee in employees)
            {
                result.Add(employee.Name);
            }
            return result;
        }  
    }

This is very simple example, but now let's extract result.Add(employee.Name) to new method.

    class EmployeeFormComponent
    {
        public IEnumerable<string> GetNames(IEnumerable<Employee> employees)
        {
            var result = new List<string>();
            foreach(var employee in employees)
            {
                NewMethod(result, employee);
            }
            return result;
        }

        private static void NewMethod(List<string> result, Employee employee)
        {
            result.Add(employee.Name);
        }
    }

Now I have new NewMethod(List result, Employee employee). It would be greate to be possible to split employee into primitive types like this.

    class EmployeeFormComponent
    {
        public IEnumerable<string> GetNames(IEnumerable<Employee> employees)
        {
            var result = new List<string>();
            foreach(var employee in employees)
            {
                NewMethod(result, employee.Name);
            }
            return result;
        }

        private static void NewMethod(List<string> result, string employeeName)
        {
            result.Add(employeeName);
        }
    }

Another possible solution is to improve extract method functionality to let select if we want to couple our method or split to primitive types.

leppie commented 8 years ago

I think Resharper does this, but refactoring cannot really do guesswork for already poor code.

You really just wanted this in the first place:

IEnumerable<string> GetNames(IEnumerable<Employee> employees) => employees.Select(x => x.Name);
anjmao commented 8 years ago

@leppie I added foreach only to show the problem, but imagine you have big loop with a lot of code. Another thing is that after roslyn was released i stopped using resharper only because I believe in roslyn and open source community :)

Pilchie commented 8 years ago

We'd definitely like to add refactorings like "Introduce parameter object", "Decompose parameter", etc, as well as signature fixes like add/remove ref/out, convert return to out and vice versa, etc.

Tagging @dpoeschl who has thought about this some already.