Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Egg Drop Dynamic Problem #400

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
kelvinlauKL merged 9 commits into kodecocodes:master from aarkalyk:master
Sep 20, 2017
Merged

Egg Drop Dynamic Problem #400

kelvinlauKL merged 9 commits into kodecocodes:master from aarkalyk:master
Sep 20, 2017

Conversation

@aarkalyk
Copy link
Contributor

@aarkalyk aarkalyk commented Mar 13, 2017

Hello!

I noticed that there aren't many dynamic programming algorithms in the repository, so I decided to contribute to the project by adding one of the most famous problems in dynamic programming "Egg Drop Problem". I'd appreciate if you could look through my code and give some feedbacks.

Thanks in advance for your attention!

Copy link
Contributor

More dynamic programming algorithms would be nice. If you take a look at the repository's pull request list, you can see a contribution for another dynamic programming algorithm which looks at the Knapsack problem.

I have a few comments at this point:

  1. To be accepted into the repository, all contributions should have a README file. As a reader, I'd want to know what problem this algorithm solves, and in general terms how it solves it.
  2. At the moment, sorry to say, this code wouldn't be accepted, because it doesn't compile. I don't know the egg-drop algorithm very well (see point 1) so I don't know how I'd correct it.
  3. The code itself needs to make its intention clearer. This would make it easier to read, easier to understand, perhaps easier to correct, and might help you write the README as well.
    For example: why is eggFloor[2][1] given a value of 1? What do the indices i, j and k represent? What's the significance of setting eggFloor[i][j] to 1000000? In particular, consider the edge-cases - would this algorithm still work correctly if the eggDrop function were called and there was only 1 floor, or if there were 1000001 floors?
    Use meaningful names; name your magic numbers; break complicated functions (e.g. the current line 17) into smaller, easier-to-digest steps; consider using optionals to represent situations where you don't yet know the value.
  4. Provide some examples of the eggDrop function - ideally at the end of a playground file and in unit tests. For example, if I wrote eggDrop(5, numberOfFloors: 3) in my code, what value would I expect it to return to show that it was working correctly?

I hope that's fair criticism and doesn't discourage you. I look forward to seeing your revised contribution.

Copy link
Member

@aarkalyk Still up for this?

Copy link
Contributor Author

@narrativium Thanks for the feedback! I'll take into account all your suggestions and submit my contribution in a few days.
@kelvinlauKL Yup!

kelvinlauKL reacted with hooray emoji

Copy link
Contributor Author

@narrativium @kelvinlauKL Please, review my pull request.
Thanks!

Copy link
Member

@aarkalyk Sorry for the wait. Going to merge this and add a few bits of polish 👍. This PR has been collecting dust long enough!

@kelvinlauKL kelvinlauKL merged commit dea3165 into kodecocodes:master Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /