Too many function arguments-I want to give you a chance to write good code. 8 [C # refactoring sample]

Problem with too many function arguments

I see code with many function arguments in various projects. There is a story that humans can understand at most seven, so please consider reducing it. The most important point is that if you think about the division of processing properly, the number of arguments will rarely increase. I wrote a sample coat based on an actual example.

Sample code

Consider getting data from eight tables on the screen and displaying it on the screen.

        void Main(int id)
        {
            //Initial display processing of the screen
            var table1Rows = GetTable1Rows(id);
            var table2Rows = GetTable2Rows(id);
            var table3Rows = GetTable3Rows(id);
            var table4Rows = GetTable4Rows(id);
            var table5Rows = GetTable5Rows(id);
            var table6Rows = GetTable6Rows(id);
            var table7Rows = GetTable7Rows(id);
            var table8Rows = GetTable8Rows(id);

            InitializeView(
                table1Rows,
                table2Rows,
                table3Rows,
                table4Rows,
                table5Rows,
                table6Rows,
                table7Rows,
                table8Rows);
        }

        /**Initial display processing**/
        private void InitializeView(
            List<Table1Row> table1Rows,
            List<Table2Row> table2Rows,
            List<Table3Row> table3Rows,
            List<Table4Row> table4Rows,
            List<Table5Row> table5Rows,
            List<Table6Row> table6Rows,
            List<Table7Row> table7Rows,
            List<Table8Row> table8Rows)
        {
            //Set to the screen.
            SetView(
                table1Rows,
                table2Rows,
                table3Rows,
                table4Rows,
                table5Rows,
                table6Rows,
                table7Rows,
                table8Rows);

        }

        /**Mysterious replacement process called in the initial display process**/
        private void SetView(
            List<Table1Row> table1Rows,
            List<Table2Row> table2Rows,
            List<Table3Row> table3Rows,
            List<Table4Row> table4Rows,
            List<Table5Row> table5Rows,
            List<Table6Row> table6Rows,
            List<Table7Row> table7Rows,
            List<Table8Row> table8Rows)
        {
            SetTable1(table1Rows);
            SetTable2(table2Rows);
            SetTable3(table3Rows);
            SetTable4(table4Rows);
            SetTable5(table5Rows);
            SetTable6(table6Rows);
            SetTable7(table7Rows);
            SetTable8(table8Rows);
        }

        /**The process of setting each DTO on the screen. The code itself to be set is omitted.**/
        private void SetTable1(List<Table1Row> table1Rows)
        {
        }

        private void SetTable2(List<Table2Row> table2Rows)
        {
        }

        private void SetTable3(List<Table3Row> table2Rows)
        {
        }

        private void SetTable4(List<Table4Row> table2Rows)
        {
        }

        private void SetTable5(List<Table5Row> table2Rows)
        {
        }

        private void SetTable6(List<Table6Row> table2Rows)
        {
        }

        private void SetTable7(List<Table7Row> table2Rows)
        {
        }

        private void SetTable8(List<Table8Row> table2Rows)
        {
        }

        /**Returning an empty instance for the sample,
         *Actually Table 1,...,Dao exists up to Table 8 and is acquired from DB.
         **/
        private List<Table1Row> GetTable1Rows(int id)
        {
            return new List<Table1Row>();
        }

        private List<Table2Row> GetTable2Rows(int id)
        {
            return new List<Table2Row>();
        }
        private List<Table3Row> GetTable3Rows(int id)
        {
            return new List<Table3Row>();
        }
        private List<Table4Row> GetTable4Rows(int id)
        {
            return new List<Table4Row>();
        }
        private List<Table5Row> GetTable5Rows(int id)
        {
            return new List<Table5Row>();
        }

        private List<Table6Row> GetTable6Rows(int id)
        {
            return new List<Table6Row>();
        }

        private List<Table7Row> GetTable7Rows(int id)
        {
            return new List<Table7Row>();
        }

        private List<Table8Row> GetTable8Rows(int id)
        {
            return new List<Table8Row>();
        }

        /** 
         *Simplified for a sample, but actually a more complex set of tables.
         *I don't think about bad table design here.
         **/
        public class Table1Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

        public class Table2Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

        public class Table3Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

        public class Table4Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

        public class Table5Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

        public class Table6Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

        public class Table7Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

        public class Table8Row
        {
            int Id { get; set; }
            string Value { get; set; }
        }

The first thing to draw is that there are two functions with eight arguments, one of which has an unknown reason for existence. I'm guessing that I probably wanted a more descriptive function name. Not only are there many arguments, but the unnecessarily long variable scope makes it difficult to keep track of how to get the arguments of the SetTable function. By the way, in the actual code, this function is not called elsewhere.

Just shorten the variable scope and it will be refreshing.

Code after refactoring

        void Main(int id)
        {
            //Initial display processing of the screen

            InitializeView(id);
        }

        /**Initial display processing**/
        private void InitializeView(int id)
        {
            var table1Rows = GetTable1Rows(id);
            SetTable1(table1Rows);

            var table2Rows = GetTable2Rows(id);
            SetTable2(table2Rows);

            var table3Rows = GetTable3Rows(id);
            SetTable3(table3Rows);

            var table4Rows = GetTable4Rows(id);
            SetTable4(table4Rows);

            var table5Rows = GetTable5Rows(id);
            SetTable5(table5Rows);

            var table6Rows = GetTable6Rows(id);
            SetTable6(table6Rows);

            var table7Rows = GetTable7Rows(id);
            SetTable7(table7Rows);

            var table8Rows = GetTable8Rows(id);
            SetTable8(table8Rows);
        }

It's not enough to solve just the number of function arguments

By the way, I will give an example with few arguments but problems.

        private List<Table1Row> _table1Rows = null;
        private List<Table2Row> _table2Rows = null;
        private List<Table3Row> _table3Rows = null;
        private List<Table4Row> _table4Rows = null;
        private List<Table5Row> _table5Rows = null;
        private List<Table6Row> _table6Rows = null;
        private List<Table7Row> _table7Rows = null;
        private List<Table8Row> _table8Rows = null;

        void Main(int id)
        {
            //Initial display processing of the screen
            _table1Rows = GetTable1Rows(id);
            _table2Rows = GetTable2Rows(id);
            _table3Rows = GetTable3Rows(id);
            _table4Rows = GetTable4Rows(id);
            _table5Rows = GetTable5Rows(id);
            _table6Rows = GetTable6Rows(id);
            _table7Rows = GetTable7Rows(id);
            _table8Rows = GetTable8Rows(id);

            InitializeView();
        }

        /**Initial display processing**/
        private void InitializeView()
        {
            //Set to the screen.
            SetView();

        }

        /**Mysterious replacement process called in the initial display process**/
        private void SetView()
        {
            SetTable1(_table1Rows);
            SetTable2(_table2Rows);
            SetTable3(_table3Rows);
            SetTable4(_table4Rows);
            SetTable5(_table5Rows);
            SetTable6(_table6Rows);
            SetTable7(_table7Rows);
            SetTable8(_table8Rows);
        }

It looks refreshing, but the bug rate is much higher. The problem is that it uses a private variable, and it's very difficult to understand when this variable was updated. In the sample, it is easy to understand because there is only one assignment location, but in most cases, a bug often occurs by updating the private variable in another event processing.

Summary

If you shorten the scope, you can shorten the argument, so please practice it. Also, if you have a lot of arguments, you should think about how to reduce them, but it is not good to use private variables. There is also a method of grouping the arguments into a class, but it is better to do it after considering the variable scope. In this case, you don't even have to put them together in a class. Note) The actual code that was the source of the sample is working and annoys the person in charge every time there is a need for investigation.

Previous article (Handling of Bool type)

Next article (Can I separate function calls and conditionals?)

Table of Contents

Recommended Posts

Too many function arguments-I want to give you a chance to write good code. 8 [C # refactoring sample]
Easy Null Check-I want to give you a chance to write good code. 6 [C # refactoring sample]
A little worrisome code collection-I want to give you a chance to write good code. 2 [C # refactoring sample]
Complicated conditional branching-I want to give you a chance to write good code. 1 [C # refactoring sample]
Bool type handling-I want to give you a chance to write good code. 7 [C # refactoring sample]
Wasteful processing of collections-I want to give you a chance to write good code. 5 [C # refactoring sample]
Is it possible to separate function calls and conditionals? --I want to give you a chance to write good code. 9 [C # refactoring sample]
A flowing interface? -I want to give you an opportunity to write good code. 3 [C # refactoring sample]
How to write good code
I want to write a nice build.gradle
I want to write a unit test!