Skip to content

Conversation

@antonszilasi
Copy link
Contributor

No description provided.

@antonszilasi antonszilasi changed the title Checkpoint sort measures alphabetically Issue #166 - Sort measures alphabetically in GUI Mar 10, 2025
@antonszilasi
Copy link
Contributor Author

Animation

antonszilasi and others added 5 commits March 14, 2025 23:04
….cpp

Co-authored-by: Dan Macumber <dan.macumber@gmail.com>
+  // std::sort(responses.begin(), responses.end(), [](const BCLSearchResult& a, const BCLSearchResult& b) {
+  //   return a.name() < b.name();
+  // });
@antonszilasi
Copy link
Contributor Author

Animation Animation

This video is still current

@macumber macumber requested a review from Copilot April 25, 2025 00:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses Issue #166 by updating the GUI to display measures in alphabetical order. Key changes include:

  • Adding a new function fetchAndSortResponses to retrieve and sort responses from the remote BCL.
  • Introducing an overall response cache (m_allResponses) and pagination logic within setTid.
  • Refactoring the setTid method to refresh responses only if the filter and search criteria change.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/shared_gui_components/BuildingComponentDialogCentralWidget.hpp Added include and declarations for the new response fetching and caching mechanism.
src/shared_gui_components/BuildingComponentDialogCentralWidget.cpp Implemented fetching, sorting, and pagination of responses; updated setTid to use the new method.

@macumber macumber marked this pull request as ready for review July 4, 2025 00:30
Comment on lines +193 to +197
if (!responses.empty()) {
std::sort(responses.begin(), responses.end(), [](const BCLSearchResult& a, const BCLSearchResult& b) {
return a.name() < b.name();
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm following correctly, you just factored a bit of code into a function, and added this bit that sorts the results.


Side note: the if (!responses.empty()) is not technically needed and does not make it faster. https://quick-bench.com/q/5lPbGyQNKctVRq7IdoD_fUVTPt4

(Running it a bunch of times locally produces results where one is sometimes faster than the other, so they're the same).

-----------------------------------------------------------
Benchmark                 Time             CPU   Iterations
-----------------------------------------------------------
SortNoIf_Items         4732 ns         4730 ns       147972
SortWithIf_Items       5040 ns         5038 ns       138705

@jmarrec jmarrec merged commit b49c5de into develop Jul 4, 2025
7 of 12 checks passed
@jmarrec jmarrec deleted the dev/anton/166 branch July 4, 2025 08:00
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants