Skip to content

Implement knapsack exercise - #1765

Merged
lemoncurry merged 19 commits into
exercism:masterfrom
sonapraneeth-a:implement-knapsack
Nov 1, 2019
Merged

Implement knapsack exercise#1765
lemoncurry merged 19 commits into
exercism:masterfrom
sonapraneeth-a:implement-knapsack

Conversation

@sonapraneeth-a

Copy link
Copy Markdown
Contributor

Implementation of knapsack exercise.

Fixes #1747


Reviewer Resources:

Track Policies

Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Currently implemented a recursive algorithm

Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
@sonapraneeth-a

sonapraneeth-a commented Oct 4, 2019

Copy link
Copy Markdown
Contributor Author

Not sure why ParallelLetterFrequencyTest is failing in build and passing in build though I did not make any changes there.

I'm currently not clear why this occurs even though I did not make any changes in that file.

ParallelLetterFrequencyTest > multipleThreadsGetUsed FAILED

Any idea on what to do further?

Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
@lemoncurry

Copy link
Copy Markdown
Contributor

Any idea on what to do further?

I think it has to do with an update today of gradle. We may need to reverse it or disable this test because it produces the following error:
java.lang.OutOfMemoryError: Java heap space

Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
@FridaTveit

Copy link
Copy Markdown
Contributor

That's unfortunate, I'm sorry it's impacted your PR @sonapraneeth-a! I'd vote for removing the parallel-letter-frequency test. It's quite a complicated test that could easily confuse users and it's not based on any canonical data. What do you think @lemoncurry? 🙂

@lemoncurry

Copy link
Copy Markdown
Contributor

@FridaTveit I agree, it should be removed.

@FridaTveit

Copy link
Copy Markdown
Contributor

I'll open a PR with it removed ASAP! 🙂

@FridaTveit

Copy link
Copy Markdown
Contributor

PR opened: #1766 🙂

@sonapraneeth-a

Copy link
Copy Markdown
Contributor Author

How do I re-run the travis build once your PR is merged @FridaTveit ?

@FridaTveit

Copy link
Copy Markdown
Contributor

@sonapraneeth-a you'll need to rebase your changes on master so that your branch has my PR on it. Here are some resources for that if you're not familiar with rebasing:

@lemoncurry

Copy link
Copy Markdown
Contributor

@sonapraneeth-a I restarted the job 😄 The check is now successful.

@lemoncurry lemoncurry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sonapraneeth-a Thank you for your contribution 😄
Concerning the algorithm you used to solve the exercise, I think there is improvement.
Otherwise only minor changes are necessary.

Comment thread config.json Outdated
"topics": [
"algorithms",
"arrays",
"control_flow_loops"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This topic is not mentioned in the topics list.
Maybe there is another topic that might fit from the official list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Recursion and dynamic programming would be suitable topics. But none of them are present in the file shared.

Comment thread config.json Outdated
"slug": "knapsack",
"uuid": "ac179b77-98a5-4daf-9773-3d68b6cd8548",
"core": false,
"unlocked_by": null,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please define an exercise that will unlock this one? e.g. saddle-points

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Currently using saddle-points to unlock knapsack

@@ -0,0 +1,2 @@
- Use recursion

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great hints 😄

Comment thread exercises/knapsack/.meta/src/reference/java/Knapsack.java Outdated
Comment thread exercises/knapsack/.meta/src/reference/java/Knapsack.java Outdated
Comment thread exercises/knapsack/.meta/src/reference/java/Knapsack.java
Comment thread config.json Outdated
Comment thread exercises/knapsack/src/main/java/Knapsack.java Outdated
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
…erence

Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
Signed-off-by: Sona Praneeth Akula <sonapraneeth.akula@gmail.com>
@sonapraneeth-a

Copy link
Copy Markdown
Contributor Author

@lemoncurry , Please review. I've made necessary changes from my side.

@lemoncurry lemoncurry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thank you @sonapraneeth-a !

@lemoncurry
lemoncurry merged commit ae8fe33 into exercism:master Nov 1, 2019
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.

knapsack: implement exercise

3 participants