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
Added code to fix issue #865 #866
Added code to fix issue #865 #866
Conversation
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.
Hello, @mamadou-diallo. Thank you for raising a PR and I appreciate your efforts in Open Source Contributions.
Here's a few things that I think you need to take care:
- Make sure to compile your program before raising a PR to avoid compile time errors.
- Always define the constants that you are using in your program.
- Take care of run time errors also(if any).
- Your code is clean. I appreciate that. Anyhow, writing clean and well structured code is always encouraged.
- Also, please document your code well.
Please make sure to address the above amd I'll be reviewing your next commits.
Thanks!
double sqrtRecursive(double num, double prev) | ||
{ | ||
double next = (prev + num / prev) / 2; | ||
if (fabs(next - prev) < DBL_EPSILON * next) |
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.
Please define the constants.
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.
@hemanth-kotagiri as requested I've added comments for the constants.
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.
Hello!
- Always compile your code before committing the changes. You have compile-time errors that you can avoid beforehand.
- Please take care of the comments I left out. I have also added what you need to do to resolve these compile-time errors. It's on your side to check for any potential run-time errors and fix them!
Thanks!
|
||
//Iterative Function | ||
double sqrtIterative(double x) { | ||
double sqrt = x * pow(0.3, log10(x)); //Square Root Value |
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.
There are multiple errors right here. You have not declared log10
or pow
anywhere in the scope. To resolve this, include the standard math library.
double sqrtRecursive(double num, double prev) | ||
{ | ||
double next = (prev + num / prev) / 2; //Smallest Square Root Value | ||
if (fabs(next - prev) < DBL_EPSILON * next) |
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.
fabs is not declared in this scope
. To resolve DBL_EPSILON is not declared in the scope
, either define a constant or write a function to return the value of DBL_EPSILON
.
|
||
cout << "Iterative Function" << endl; | ||
cout << "Please enter a number" << endl; | ||
double iFn; |
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.
Also, please have elaborate and meaningful variable declarations. Change the same wherever applicable.
|
||
//Recursive Function | ||
cout << "Recursive Function" << endl; | ||
cout << "Please enter 2 numbers, the first will be the square root number" << endl; |
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.
To demonstrate the recursive algorithm, instead of asking the user for a second number, you could define another constant or even generate a random number within a range specified and pass it as an argument to your sqrtRecursive()
function thereby asking the user only to specify a single number that which they desire to find the square root of.
Please assign it to me |
Pull Request Template
What have you Changed(must)
Created Iterative and Recursive Function to fix issue #865
Issue no.(must) - #865
Pr will be closed and marked as spam. If issue number not found or issue was assigned to someone else.
Marking as spam can block your account from HacktoberFest.
Self Check(Tick After Making pull Request)
README - How to Contribute