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

feat: add Knapsacp algorithm in dynamic algorithm #1277

Closed

Conversation

aa001R
Copy link

@aa001R aa001R commented Jun 13, 2023

Description of Change

Hi. I have added the knapsack(0-1 knapsack)algorithm code to dynamic programming.

References

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • 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:

@Panquesito7 Panquesito7 added the enhancement New feature or request label Jun 13, 2023
@aa001R
Copy link
Author

aa001R commented Jun 13, 2023

@Panquesito7
To solve this issue,
(

  • 1 configuration not found
  • Warning: Code scanning cannot determine the alerts introduced by this pull request
    )
    I have updated the codeql.yml file.

.github/workflows/codeql.yml Outdated Show resolved Hide resolved
@aa001R
Copy link
Author

aa001R commented Jun 14, 2023

@Panquesito7
Thank you for your feedback !
I updated the codeql.yml file again.

Copy link
Member

@madhavgaba madhavgaba left a comment

Choose a reason for hiding this comment

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

Nit: rename the vars

dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
alexpantyukhin
alexpantyukhin previously approved these changes Jul 5, 2023
Copy link
Collaborator

@realstealthninja realstealthninja 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! ❤️

dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
aa001R and others added 2 commits July 15, 2023 20:32
@aa001R
Copy link
Author

aa001R commented Jul 15, 2023

HI @alexpantyukhin

I wanted to inform you that I've made some updates, which automatically dismissed the previous approval. The latest commit (commit hash: 8ca58f0) includes changes, such as adding comments for clarification.

I kindly request your review once again to provide your valuable feedback on the latest changes.

Thank you and best regards :)!

Copy link
Collaborator

@realstealthninja realstealthninja left a comment

Choose a reason for hiding this comment

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

This looks absolutely wonderful! ❤️

dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
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.

Looks like there are a lot of errors here: https://github.com/TheAlgorithms/C/actions/runs/5652143546/job/15350288419?pr=1277#step:3:672
Please fix those. Let us know if you need any help with that. 🙂

@aa001R
Copy link
Author

aa001R commented Jul 27, 2023

Hi @Panquesito7

I noticed that in line 36-37 of the 'Item' struct, direct member initialization within the struct is not supported in standard C language. so fixed that line.

Thank you :)!

Panquesito7
Panquesito7 previously approved these changes Jul 27, 2023
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.

Awesome. Thank you! 🚀

alexpantyukhin
alexpantyukhin previously approved these changes Aug 1, 2023
Copy link
Collaborator

@realstealthninja realstealthninja left a comment

Choose a reason for hiding this comment

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

Looks great just a small nitpick

dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
dynamic_programming/knapsack.c Outdated Show resolved Hide resolved
@aa001R aa001R dismissed stale reviews from alexpantyukhin and Panquesito7 via fa39b11 August 14, 2023 01:33
Copy link
Collaborator

@realstealthninja realstealthninja left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@aa001R
Copy link
Author

aa001R commented Sep 11, 2023

Hi @tjgurwara99,

Could you please review this PR, when you have a moment ? Your attention to this matter is greatly appreciated.

Thank you:)

@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
Copy link
Contributor

Please ping one of the maintainers once you commit the changes requested or make improvements on the code. If this is not the case and you need some help, feel free to ask for help in our Gitter channel or our Discord server. Thank you for your contributions!

@github-actions github-actions bot closed this Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants