Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code Duplication #22

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Conversation

EndermanPC
Copy link
Member

There’s some code duplication in your main function where you’re calling "LoadDynamicLibrary" with different parameters based on the platform. You could define platform-specific constants for the library name and use them in a single call to "LoadDynamicLibrary".

There’s some code duplication in your main function where you’re calling "LoadDynamicLibrary" with different parameters based on the platform. You could define platform-specific constants for the library name and use them in a single call to "LoadDynamicLibrary".
@Andrej123456789
Copy link
Member

Thanks a lot for pull request. Can you please check duplicates in Termi-GUI?

@Andrej123456789 Andrej123456789 added bug Something isn't working C++ C++ area labels Apr 30, 2024
try {
auto func = LoadDynamicLibrary<int(*)(int)>(LIB_NAME, "tmain");
func(12);
} catch (const std::exception& e) {
Copy link
Member

Choose a reason for hiding this comment

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

Please remove part of code implementing exceptions, because they are not unnecessary due to following:
a) this is function from C library where exceptions do not exist,
b) errors when loading function is handled in LoadDynmaicLibrary function.

Rest of code is excellent, thank you for this idea!

@EndermanPC EndermanPC closed this Sep 25, 2024
@Andrej123456789 Andrej123456789 merged commit 3e85bbe into ringwormGO-organization:main Sep 27, 2024
Andrej123456789 added a commit that referenced this pull request Sep 27, 2024
@EndermanPC EndermanPC deleted the patch-1 branch October 31, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C++ C++ area
Projects
Development

Successfully merging this pull request may close these issues.

2 participants