Skip to content

Checks input of factorial#220

Closed
dylanhernandez1 wants to merge 2 commits intoTheAlgorithms:masterfrom
dylanhernandez1:master
Closed

Checks input of factorial#220
dylanhernandez1 wants to merge 2 commits intoTheAlgorithms:masterfrom
dylanhernandez1:master

Conversation

@dylanhernandez1
Copy link
Copy Markdown

@dylanhernandez1 dylanhernandez1 commented Apr 6, 2023

This change simply just adds a feature to the factorial part to make sure that the input is valid. Specifically, it makes sure that the "n" variable input is valid by checking if it is greater than or equal to zero and if it is indeed a whole number. If it is not, the program prints that "n" is an invalid input. I agree to all of the terms below.

Welcome to Dart community

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?
  • Add/Update/Fix test cases.

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Dart files are placed inside an existing directory.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Makes sure that the "n" variable input is valid by checking if it is greater than or equal to zero and if it is indeed a whole number. If it is not, the program prints that "n" is an invalid input.
Comment thread maths/factorial.dart Outdated
void main() {
var n = 5;
var fac = factorial(n);
if(n>=0 && n.runtimeType == int){
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of checking types, Why not replace var n =5; with int n=5;
This will help us avoid the unnecessary type checking.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We can do that, I would just say to leave it a variable in case if user input is needed. Whatever you want though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since we are not taking any user input and dart has a great types. let's exploit that.

you can add test for the same.

Comment thread maths/factorial.dart
Comment on lines +4 to +8
var fac = factorial(n);
print("$n! = $fac"); /* output: 5! = 120 */
}else{
print("Invalid input. Input has to be a positive whole integer");
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you add more tests to this file. Checkout other files to see how tests are written there.
Two sum File

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's remove these print statements and have proper tests for these functions. im in the process of upgrading the dart version and adding tests to old files so we can migrate to null safety and know all algorithms are working fine with Null safety.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You want me to put more tests in there for the factorials? Are they the same as the ones that you put on the two sum file?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Currently we are just having print statements. Instead of prints. Let's have tests which are more effective. I have shared the file for you reference. you can check some other files in the repo as well to get better clarity on writing tests.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I can't think of any tests to add, sorry...let me know if there are specific tests I can add

@akashgk akashgk self-assigned this Apr 12, 2023
@akashgk akashgk closed this Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants