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

test: Add self-test cases in the math/power_of_two.cpp file #1640

Merged
merged 5 commits into from
Feb 10, 2022

Conversation

Rijul24
Copy link
Contributor

@Rijul24 Rijul24 commented Oct 2, 2021

Description of Change

<!
//* Added Self Implementation cases
//* Necessary changes for user inputs
//* Changed necessary comments

Thank you for your Pull Request. Please provide a description above and review
the requirements below.

Contributors guide: https://github.com/TheAlgorithms/C-Plus-Plus/CONTRIBUTING.md
-->

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
  • Relevant documentation/comments is changed or added
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • I acknowledge that all my contributions will be made under the project's license.

Notes:

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Great work! 😄👍

Comment on lines 24 to 25
//Added self implementation Cases Contribution

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//Added self implementation Cases Contribution

Comment on lines 27 to 28
#include <cassert> ///for assert

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#include <cassert> ///for assert
#include <cassert> /// for assert

Comment on lines 45 to 46
* returns 1 if n is power of 2
* returns 0 if n is not a power of 2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* returns 1 if n is power of 2
* returns 0 if n is not a power of 2
* prints the result, as "Yes, the number n is a power of 2" or
* "No, the number is not a power of 2" without quotes
* @returns 1 if `n` IS the power of 2
* @returns 0 if n is NOT a power of 2

Comment on lines 51 to 54

if(result == 0) return 1 ; //yes it is
else return 0; // no it is not

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(result == 0) return 1 ; //yes it is
else return 0; // no it is not
if (result == 0) {
std::cout << "Yes, the number " << n << " is a power of 2";
} else {
std::cout << "No, the number " << n << " is not a power of 2";
}

assert(math::power_of_two(232) == 0);
std::cout << "Passed\n";

std::cout << "All test cases passed! \n";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::cout << "All test cases passed! \n";
std::cout << "All test cases passed!\n";


test(); //run self-test implementations

//UN - COMMENT BELOW LINE TO TAKE USER INPUTS
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//UN - COMMENT BELOW LINE TO TAKE USER INPUTS
// uncomment the line below to take user inputs

/**
* @brief Main function
* @returns 0 on exit
*/

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

math/power_of_two.cpp Show resolved Hide resolved
* @brief take user input
* @returns void
*/

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

}

/**
* @brief take user input
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @brief take user input
* @brief Take user input in the test cases (optional; currently commented)

@Panquesito7 Panquesito7 added the Improvement improvement in previously written codes label Oct 10, 2021
@Panquesito7 Panquesito7 changed the title Added self-implementation test cases in power_of_two.cpp test: Added self-implementation test cases in power_of_two.cpp Oct 10, 2021
@Panquesito7 Panquesito7 changed the title test: Added self-implementation test cases in power_of_two.cpp test: Add self-test cases in the math/power_of_two.cpp file Oct 10, 2021
@github-actions
Copy link
Contributor

This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Author has not responded to the comments for over 2 weeks label Jan 10, 2022
@Panquesito7 Panquesito7 removed the stale Author has not responded to the comments for over 2 weeks label Jan 10, 2022
@github-actions
Copy link
Contributor

This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Author has not responded to the comments for over 2 weeks label Feb 10, 2022
@Panquesito7 Panquesito7 removed the stale Author has not responded to the comments for over 2 weeks label Feb 10, 2022
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thank you for your contribution! 😄👍

@Panquesito7 Panquesito7 added the approved Approved; waiting for merge label Feb 10, 2022
@Panquesito7 Panquesito7 merged commit 27ced49 into TheAlgorithms:master Feb 10, 2022
Copy link
Member

@CarlosZoft CarlosZoft left a comment

Choose a reason for hiding this comment

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

thanks for contributing 🔥

if (result == 0) {
std::cout << "Yes, the number " << n << " is a power of 2";
return 1;
} else {
Copy link
Member

Choose a reason for hiding this comment

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

'else' is redundant in this case

Copy link
Member

Choose a reason for hiding this comment

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

Hey there! Want to make a PR to fix this?

Copy link
Member

Choose a reason for hiding this comment

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

yes! i can do this

Copy link
Member

Choose a reason for hiding this comment

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

done! #1936

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved; waiting for merge Improvement improvement in previously written codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants