Refactor `Utils::Gui::openFolderSelect` to use `switch-case` #16910

New issue

Open

opened 2026-02-22 03:31:36 -05:00 by deekerman · 4 comments

deekerman commented

2026-02-22 03:31:36 -05:00

Owner

Originally created by @fkobi on GitHub (May 25, 2025).

Suggestion

Right now the title's function uses 5 ifs and one default behaviour:
github.com/qbittorrent/qBittorrent@6418033cc8/src/gui/utils.cpp (L182-L210)
Maybe it just grew overtime.

I think it is worth it to rewrite it using switch-case.
It improves readability (and efficiency).

Extra info

I am not sure if this is a feature but this is not a bug.
I would call it an improvement idea.

I came upon this by reading commit github.com/qbittorrent/qBittorrent@6418033cc8 from patch notes.

Originally created by @fkobi on GitHub (May 25, 2025). ### Suggestion Right now the title's function uses 5 `if`s and one default behaviour: https://github.com/qbittorrent/qBittorrent/blob/6418033cc8d4ec2f31679418cb480af498dafb2c/src/gui/utils.cpp#L182-L210 Maybe it just grew overtime. I think it is worth it to rewrite it using `switch-case`. It improves readability (and efficiency). ### Extra info I am not sure if this is a feature but this is not a bug. I would call it an improvement idea. I came upon this by reading commit https://github.com/qbittorrent/qBittorrent/commit/6418033cc8d4ec2f31679418cb480af498dafb2c from patch notes.

deekerman added the

Feature request

Code cleanup

labels

2026-02-22 03:31:36 -05:00

deekerman commented

2026-02-22 03:31:44 -05:00

Author

Owner

@HanabishiRecca commented on GitHub (May 26, 2025):

I am not sure if this is a feature but this is not a bug.
I would call it an improvement idea.

PRs are welcome.

@HanabishiRecca commented on GitHub (May 26, 2025): > I am not sure if this is a feature but this is not a bug. > I would call it an improvement idea. PRs are welcome.

deekerman commented

2026-02-22 03:31:44 -05:00

Author

Owner

@glassez commented on GitHub (May 26, 2025):

What are you guys talking about? It's C++. switch supports only integral types and enumerations.

@glassez commented on GitHub (May 26, 2025): What are you guys talking about? It's C++. `switch` supports only integral types and enumerations.

deekerman commented

2026-02-22 03:31:44 -05:00

Author

Owner

@HanabishiRecca commented on GitHub (May 26, 2025):

That's why I said to send a PR. For the reporter to just try to implement it, instead of weird suggestions.
And maybe would come up with a better idea in process, who knows.

@HanabishiRecca commented on GitHub (May 26, 2025): That's why I said to send a PR. For the reporter to just try to implement it, instead of weird suggestions. And maybe would come up with a better idea in process, who knows.

deekerman commented

2026-02-22 03:31:44 -05:00

Author

Owner

@fkobi commented on GitHub (May 26, 2025):

We are comparing output to a compile-time-known UTF strings -- I think enum can be used.

When I finally get some spare time I'll make a PR but it does not seem like that will happen soon.

@fkobi commented on GitHub (May 26, 2025): We are comparing `output` to a compile-time-known UTF strings -- I think `enum` can be used. When I finally get some spare time I'll make a PR but it does not seem like that will happen soon.