A 200-line function is given that processes a large amount of data and takes many parameters. By incredible efforts, the function was cut into 4 separate functions, but the following problem appeared: each of the functions had an equally large list of parameters. If I suddenly want to change something, then I have to correct the list of parameters in all 4 functions, and not in one + the total solution is already 300 lines. What can be done in this situation? What refactoring to use?

Original signature (identical to natural):

template<typename value_type, typename some_other_type> auto original_foo(const std::vector<value_type>& data_1, const std::map<int, some_other_type>& data_2, int param_1, int param_2, std::pair<int, int>& indirect_result) { //200 long long lines... }

My attempt to refactor degenerates into these 4 functions:

 template<typename value_type, typename some_other_type> auto solve_1_task(const std::vector<value_type>& data_1, const std::map<int, some_other_type>& data_2, int param_1, int param_2, std::pair<int, int>& indirect_result) { //50 lines... solve_2_task(data_1, data_2, param_1, param_2, indirect_result); }
  • Unit tests, I hope, have written before refactoring? - Vladimir Gamalyan
  • @VladimirGamalian if I knew what this function should do - KPetrov
  • @VladimirGamalian and this is not a royal business - KPetrov
  • 2
    Then, perhaps, it is better not to touch anything;) - Vladimir Gamalyan

3 answers 3

If you do the right refactoring, then following the principle of Single Responsibility when describing methods, the number of arguments rarely exceeds 1, but you should not rest on this recommendation turning off the logic and breaking all common sense.

I see that the processing of data in each function affects all your data. For a start, look, is it possible to somehow process the data separately? You broke a large function into a small one, but the algorithm remains the same, maybe there is an opportunity not to affect all the data? After all, what you have done has not changed the essence of the problem ...

We treat your code:

Rethinking functions:

  1. A function should ideally perform only one operation. She should do it well and she should do nothing else. To make sure that the function "performs only one operation", it is necessary to check that all the commands of the function are at the same level of abstraction .

Variable refactoring:

If a function should receive more than two or three arguments, it is very likely that some of these arguments should be packaged in a separate class. Consider the following two announcements:

 Circle makeCircle(double x, double y, double radius); Circle makeCircle(Point center, double radius)

Reducing the number of arguments by creating objects may seem like a scam, but it is not. If variables are transmitted together as a unit (as variables x and y in this example), then, most likely, together they form a concept that deserves its own name.

Next, you need to see if it is impossible to perform any actions in the class itself with the parameters passed in order not to perform them in a function?

Some variable refactoring rules

  1. If the data passed to the method can be obtained by calling the method of another object, we use the replacement of the parameter by calling the method. This object can be placed in the field of its own class or passed as a parameter of the method.

  2. Instead of transferring a group of data received from another object as parameters, the object itself can be transferred to the method using the transfer of the entire object.

  3. If there are several unrelated data elements, sometimes they can be combined into a single parameter object, applying the replacement of parameters with an object. Suppose you passed a start, end to a function generating a report, and now just pass the DateRange object, where there are start, end parameters.

There are a lot of ways to refactor your code and it depends on the code itself and the task you are performing. It’s hard to say something when you don’t see the code, you should read the code refactoring books, for example, Clean Code. Creating, analyzing and refactoring R. Martin or the book By design patterns, to properly distribute all classes.

    I refer to the authority :) But first - are you not just mechanically dividing the function into four? Too it hints at the fact that all parameters are needed in all functions ...

    Well, now - the word Fowler. On the long lists of parameters, he writes:

    In the long lists of parameters is difficult to understand. They are often controversial and difficult to apply, and in addition, they have to change forever as new data become necessary. If, however, you pass objects to methods, then you will need fewer changes, since a couple of requests will most likely suffice to obtain new data.

    Refactoring “Replacing a parameter with a method call” is suitable when you can get data in one parameter by calling the method of an object that is already known. This object can be a field or another parameter. Refactoring “Saving the entire object” allows you to replace the set of data received from the object with the object itself. If there are a number of data elements without a logical object, you can use the “Introduction of the parameter object” refactoring.

    There is one important exception when such changes should not be made. This is the case when we do not want to create an explicit relationship between the callee and the larger objects. In such cases, it is reasonable to unpack the data and transfer them separately as parameters, but be aware of the cost of this solution. If the parameter list is too long or the modifications are too frequent, it is better to revise the dependency structure.

    • @KPetrov, creating a class for transmitting a long list of parameters, is reminiscent of dust sweeping under the rug, and in my opinion only complicates the future life of the project. On good, to refactor here for a start it is necessary the code which causes "problem" function. - avp
    • @avp As for me - there is one exception :) - if all these parameters are interconnected, so in fact they are more like aspects of something whole ... Combining the mechanically non-united is, of course, only making it worse. - Harry

    Why are consecutive task calls chained?
    It would be better to call your four tasks from the original function, sequentially selecting some code into a separate function:

     original_foo(params) { solve_1_task(params); solve_2_task(params); ... }

    About the parameters:
    If parameters are really needed everywhere, the best solution might be to allocate this functionality to the class.

    Just pass all the parameters to the class constructor, and use the class members in the methods.

     original_foo(params) { Foo<value_type, some_other_type> foo(params); foo.solve_1_task(); ... }