Bug 155628 - Split huge complex functions into multiple functions with less complexity
Summary: Split huge complex functions into multiple functions with less complexity
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
7.6.0.0 alpha1+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyBeginner, easyHack, skillCpp, skillJava, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2023-06-01 13:39 UTC by Hossein
Modified: 2023-06-12 07:16 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hossein 2023-06-01 13:39:00 UTC
Description
Some of the LibreOffice modules have functions that contain hundreds of lines of code. They are complex, hard to understand, change and maintain. This is the task to find such complex functions, and split them into multiple functions according to the different logical parts of work.

Rationale
Huge functions, with Line of code (LOC) > 100 are hard to work with. There are measures of complexity beyond line of code, like having conditions, loops, gotos, and nested functions. To make the life of the developers a little easier, breaking the complex functions into multiple functions can be helpful.

Finding complex functions
There are many tools that can help doing this. For example, clang-tidy has a built-in check (since 2017) that can rate functions according to a complexity metric, and report those that are more complex than a certain threshold:

clang-tidy - readability-function-cognitive-complexity
https://clang.llvm.org/extra/clang-tidy/checks/readability/function-cognitive-complexity.html

For example, invoke this command, shows 4 functions with the complexity measure of > 100. The most complex one is "void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)" with the complexity of 645.

clang-tidy --config="{Checks: '-*, readability-function-cognitive-complexity',CheckOptions: [{key: readability-function-cognitive-complexity.Threshold, value: 100},{key: readability-function-cognitive-complexity.DescribeBasicIncrements, value: false} ]}" sw/source/core/layout/tabfrm.cxx

You can use this tool via command line, or via an IDE. For example, Qt Creator is accompanied with this tool, and it also provides a GUI tool for using it on a certain folder of the source code. You can find the clang-tidy binary in:
/opt/qtcreator-10.0.0/libexec/qtcreator/clang/bin/clang-tidy

Changing the code
To break the function into multiple functions, the best would be doing the refactoring in a way that:
1. One logical unit of work is done in each function.
2. Sane names are used for each function that describe the their functionality
3. The parameter passing is minimized
4. Clean coding principles are met
5. The resulting functions should be short (less than ~50-100 LOC), with much lower complexity

Important notes:
1. You have to make sure that the new code has the same functionality as the old one, and it does not cause performance impacts. Of course you are free to fix some bugs later. :-)

2. Work on only part of a single module, because there can be many results. Running the above command on sw/source/core/layout/*.cxx gives multiple results.

3. Changing the largest functions can be the most useful thing to do, but it will not be easy. So, you may start from the somehow less complex ones.
Comment 1 danomois 2023-06-11 15:17:53 UTC Comment hidden (irrelevant, obsolete)
Comment 2 Hossein 2023-06-12 07:16:20 UTC
Setting to new as this EasyHack is discussed in the ESC call:

ESC meeting minutes: 2023-06-01
https://lists.freedesktop.org/archives/libreoffice/2023-June/090430.html