-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Update AllCombinationsOfSizeK.js #1530
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
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.
Looks good, however formatting seems broken. If you're already refactoring this, could you get rid of the class entirely? This should be a single function taking n
and k
as parameters instead. Ideally the API should also be a generator rather than producing a list.
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.
You'll need to fix the tests, though. They currently still expect a class.
Changes made it the type of testing. Instead of testing the class now the program will test the function
I am not able to get that why there is code style error in this pull request |
Describe your change:
This PR modifies a current algorithm for AllCombinationsOfSizeK. Four parameters made up the original findCombinations() method, and they were duplicated for each stack call. Since the modifications are kept in the instance variables, the modified findCombinations() method does not require any parameters. In this case, there will be far fewer local variables that need to be initialized overall on each recursive call.
There was also a for loop in the original technique. The updates function is entirely recursive and lacks a for loop.
Checklist:
Example:
UserProfile.js
is allowed butuserprofile.js
,Userprofile.js
,user-Profile.js
,userProfile.js
are notFixes: #{$ISSUE_NO}
.