-
-
Notifications
You must be signed in to change notification settings - Fork 130
new snippet - swap numbers #172
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
new snippet - swap numbers #172
Conversation
Hey, Thank you for your contribution.
After reviewing your PR we've found that all/some snippets you are adding don't match the scope of Quicksnip.
The ability to convert a string to an int in c++ is already provided by std::to_string.
Please remove the PR that don't match the scope (Listed bellow), if your PR contains no snippet matching the scope of Quicksnip, it will be closed without further notice.
If no fix is provided within 7-10 days this PR will be closed without further notice
It is a snippet that converts a string to integer. Built in function for string to int would be std::to_integer() or stoi() with complexity O(N), my snippet is basically the same. Should I update it to use the built in function?
Oh sorry, the title is a bit miss leading... if an std function already exists to do that a snippet isnt necessary
i have another snippet idea can i make the changes in this pr itself or should i open a new one... the new snippet would be swap 2 nums without third variable.. any suggestions abt this?
Add it here, we will see if it fits as a snippet in quicksnip
done...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, Thank you for your contribution! π However, we can't currently accept your snippet:
The arithmetic swapping logic is redundant since the function is already returning a tuple. You can achieve the same result more efficiently by directly swapping the values.
I believe it could fit in the C categories, where there is no std::pair or std::tuple. You could take the parameters as pointer and achieve the same result.
Let me know if you have any questions or need further clarification.
Could you please update the name of your PR ? It would make it more easier to track.
updated the colon as well simple swapping using pointers in C
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it's good, could you however format your code, it seems really compact. Here's an example:
*num1 = *num1 + *num2; int a = 3, b = 4; swap(&a, &b);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto is a C++ keyword.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's seems fine for me, however I want a 2nd opinion, could you please wait for another maintainers to review your snippet
works for me thanykou!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite like the comment here, i would prefer something along the lines of Swaps the values of the a and b variables
Appart from that it looks good
done
621db73
into
quicksnip-dev:main
Description
Type of Change
Checklist
Related Issues
Closes #
Additional Context
Screenshots (Optional)
Click to view screenshots