Skip to content

fixes #110 #148

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

Merged
merged 4 commits into from
Dec 14, 2021
Merged

fixes #110 #148

merged 4 commits into from
Dec 14, 2021

Conversation

Swastyy
Copy link
Contributor

@Swastyy Swastyy commented Nov 12, 2021

fixes #110

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.

Good work! 😄👍

Panquesito7
Panquesito7 previously approved these changes Nov 12, 2021
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! 😄👍

Copy link
Collaborator

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

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

Your complexity considerations seem off. You forget to elaborate on how the item with the highest efficiency is found (a naive implementation might take quadratic time, a more advanced one would use something like a heap or even a median-based algorithm).


Given a set of items, each with weight and a value, determine the number of each item included in a collection so that the total weight is less than or equal to the given limit and the total value is as large as possible.

##### Greedy method will always provide an optimal solution with fractional knapsack problem.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not supposed to be a heading. Use a different way of emphasis. Please explicitly state that it often finds a suboptimal solution for nonfractional knapsack problems.


O(1) For iterative approach
O(1) For recursive approach *if tail call optimization is used*, O(log n) due to recursion call stack, otherwise
O(nlog n) Worst Case
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing a space. Also only the case if you presort the items (which should definitely be mentioned!). Also note that it can be done in O(n) using medians.

@github-actions
Copy link

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 label Dec 14, 2021
@vbrazo vbrazo merged commit addcea6 into TheAlgorithms:master Dec 14, 2021
@appgurueu
Copy link
Collaborator

My comments were not resolved.

@Panquesito7 Panquesito7 removed the stale label Dec 14, 2021
@Panquesito7
Copy link
Member

Hey, @Swastyy! If you have time, could you please address @appgurueu's comments in another PR? Let us know if you need any help! Thanks. 😃

@Swastyy Swastyy deleted the greedy branch December 15, 2021 04:31
@Swastyy
Copy link
Contributor Author

Swastyy commented Dec 15, 2021

Yeah, I'll do it in another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Addition of folder for greedy algorithms explanations in TheAlgorithms/Algorithms-Explanation/en/
4 participants