Bug 153294 - Use copy_if, find_if or remove_if to simplify loops
Summary: Use copy_if, find_if or remove_if to simplify loops
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.4 all versions
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:25.8.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2023-01-31 15:14 UTC by Hossein
Modified: 2025-07-30 08:02 UTC (History)
3 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-01-31 15:14:44 UTC
Since C++11, STL provides means to process containers easier than before via the available functions inside <algorithm>.

In tdf#153109, I have suggested using std::all_of, std::any_of and std::none_of.

Here, I suggest to simplify loops via std::copy_if, find_if or remove_if:

std::copy_if
https://cplusplus.com/reference/algorithm/copy_if/

std::find_if
https://cplusplus.com/reference/algorithm/find_if/

std::remove_if
https://cplusplus.com/reference/algorithm/remove_if/

In the above links, you can find description for each of the above functions. Beyond that, you can find good tutorials for these functions. For example:

Different methods to copy in C++ STL | std::copy(), copy_n(), copy_if(), copy_backward()
https://www.geeksforgeeks.org/different-methods-copy-c-stl-stdcopy-copy_n-copy_if-copy_backward/

For an example of such changes inside LibreOffice code, see this commit:

Simplify Sequence iterations in lingucomponent..lotuswordpro 760a377f7148e623e9e16d24e66f54a401ecb946

You have to make sure that you keep the code behavior unchanged when using the above STD functions.
Comment 1 Radhey Parekh 2023-02-03 16:50:52 UTC
I would like to work on this as I've implemented this in one of my patches. Also, can you please provide me the code pointer to start on? Thanks.
Comment 2 Buovjaga 2023-03-24 14:02:52 UTC
(In reply to Radhey Parekh from comment #1)
> I would like to work on this as I've implemented this in one of my patches.
> Also, can you please provide me the code pointer to start on? Thanks.

You can use git grep to find similar patterns throughout the codebase.
Comment 3 Aron Budea 2024-03-24 23:03:18 UTC
This is a task that multiple people can work on at the same time, no need to assign it to a single person.
Comment 4 Commit Notification 2025-04-28 01:05:50 UTC
Simon Chenery committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d9435d2759e78b716b64bfccdecaac3f3f1a0e4d

tdf#153294 simplify for loop to std::find call in ImplHelper.cxx

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 5 Commit Notification 2025-05-13 13:11:03 UTC
Rafał Dobrakowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/b0a9be01890578d239bcb958dec33c301986519d

tdf#153294 Optimize item index retrieval in Menu::GetCharacterBounds()

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Rosalio Morales 2025-07-29 23:46:55 UTC
I have submitted a patch for this issue to Gerrit for review:

https://gerrit.libreoffice.org/c/core/+/188567

This patch simplifies two loops using std::transform and back_inserter in rangecache.cxx. It keeps the original logic intact while improving readability and modernizing the code.

More improvements related to this EasyHack will follow in subsequent patches.
Comment 7 Rosalio Morales 2025-07-30 02:34:34 UTC
I have submitted a patch that simplifies the loops in ScSortedRangeCache using std::transform and std::back_inserter, improving readability and aligning with modern C++ practices.

Gerrit change: https://gerrit.libreoffice.org/c/core/+/188567

This patch replaces repeated for-loops in rangecache.cxx (both for numeric and string values) with concise and efficient STL-based transformations.

Marking this bug as ASSIGNED.

Thanks!
Comment 8 Rosalio Morales 2025-07-30 04:32:31 UTC
Reviewed remaining for-loops in sc/source/core/tool/rangecache.cxx.

The loops that populate mRowToIndex and mColToIndex use explicit indexing to assign values into pre-sized vectors.

These cannot be safely or cleanly replaced by STL algorithms (like std::transform) because they rely on both the index and the value simultaneously.

Therefore, these for-loops were intentionally left unchanged to preserve clarity and performance.

All convertible loops in this file have been updated or reviewed.
Comment 9 Rosalio Morales 2025-07-30 04:55:54 UTC
Reviewed all remaining for-loops in rangecache.cxx.
The two loops that populate mRowToIndex and mColToIndex rely on the loop index to assign into pre-sized vectors.
These are not appropriate for STL algorithms like std::transform.
Added explanatory comments and left the logic as is.

Patch submitted to Gerrit: https://gerrit.libreoffice.org/c/core/+/I35b084e706fd379c8f113e4b78a76c760d40bc7b
Comment 10 Buovjaga 2025-07-30 05:40:30 UTC
(In reply to Rosalio Morales from comment #7)
> Marking this bug as ASSIGNED.

As discussed, multi-hacker tasks are not assigned.
Comment 11 Rosalio Morales 2025-07-30 08:01:36 UTC
A new patchset has been uploaded for review:

https://gerrit.libreoffice.org/c/core/+/188570

This patch simplifies loops in ScSortedRangeCache using std::transform and back_inserter, improving readability while maintaining logic.

Set to "Assigned" and confirming I'm working on this issue.
Comment 12 Rosalio Morales 2025-07-30 08:02:07 UTC
Patch submitted for review:
https://gerrit.libreoffice.org/c/core/+/188570

This patch simplifies the for-loops in ScSortedRangeCache using std::transform and back_inserter.

Looking forward to feedback. I'm continuing to work on related cleanup in the same file as part of tdf#153294.