Skip to content

added kadane algorithm function #1079

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.

Sign up for GitHub

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

Jump to bottom
Merged
kvedala merged 31 commits into from
Sep 23, 2020
Merged

added kadane algorithm function #1079

kvedala merged 31 commits into from
Sep 23, 2020

Conversation

@ayush523
Copy link
Contributor

@ayush523 ayush523 commented Sep 15, 2020 •
edited by gitpod-io bot
Loading

@Panquesito7 Panquesito7 added automated tests are failing Do not merge until tests pass enhancement New feature or request Proper Documentation Required requested to write the documentation properly labels Sep 15, 2020
@Panquesito7 Panquesito7 linked an issue Sep 15, 2020 that may be closed by this pull request
[FEATURE]Create Kadane's Algorithm #1038
Closed
@ayush523
Copy link
Contributor Author

ayush523 commented Sep 15, 2020

What is the problem with the file that I have added?Please help

Panquesito7
Panquesito7 requested changes Sep 15, 2020
View reviewed changes
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.

Please read the Contribution Guidelines and fix clang-tidy warnings.

@Panquesito7 Panquesito7 added the requested changes changes have been requested label Sep 15, 2020
@Panquesito7
Copy link
Member

Panquesito7 commented Sep 15, 2020

The file does not need to be in a specific folder for this algorithm.
It can be in the others folder, or in its appropriate folder. 🙂

@ayush523
Copy link
Contributor Author

ayush523 commented Sep 16, 2020 via email

@ayush523
Copy link
Contributor Author

ayush523 commented Sep 16, 2020

I have made the changes.Please check.

@Panquesito7 Panquesito7 removed the automated tests are failing Do not merge until tests pass label Sep 16, 2020
Panquesito7
Panquesito7 requested changes Sep 16, 2020
View reviewed changes
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 much better, good work. 😄 👍
Now, the next thing we shall do is to add documentation.

Confused? See #962 as a reference. 🙂

@ayush523
Copy link
Contributor Author

ayush523 commented Sep 16, 2020

Thanx. I did the changes which you suggested.

Panquesito7
Panquesito7 reviewed Sep 16, 2020
View reviewed changes
@ayush523
Copy link
Contributor Author

ayush523 commented Sep 16, 2020

Any more changes required?

@Panquesito7
Copy link
Member

Panquesito7 commented Sep 16, 2020

Yes, please add documentation like in #962.

Panquesito7
Panquesito7 reviewed Sep 16, 2020
View reviewed changes
@ayush523 @Panquesito7
Update kadane_algorithm/kadane.cpp
53549e4 
Co-authored-by: David Leal <halfpacho@gmail.com>
Panquesito7
Panquesito7 requested changes Sep 16, 2020
View reviewed changes
ayush523 and others added 3 commits September 17, 2020 10:57
@ayush523 @Panquesito7
Update kadane_algorithm/kadane.cpp
79610e2 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update kadane_algorithm/kadane.cpp
90d1c91 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523
added details in the documentation
09f4986
Panquesito7
Panquesito7 requested changes Sep 18, 2020
View reviewed changes
ayush523 and others added 10 commits September 19, 2020 00:02
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
a48b8e7 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523
changes
fb0c7a9
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
c545de3 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
31ca982 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
233c15d 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
26346a6 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
b76f494 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
6e0fe4c 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523
added code
1883b5c
@ayush523
change
e94e39e
@ayush523
Copy link
Contributor Author

ayush523 commented Sep 19, 2020

@Panquesito7 -I don't understand why the code is not working after the commits suggested.Please help.

Panquesito7
Panquesito7 requested changes Sep 20, 2020
View reviewed changes
Panquesito7
Panquesito7 requested changes Sep 20, 2020
View reviewed changes
ayush523 and others added 4 commits September 20, 2020 22:31
@ayush523 @Panquesito7
Update .vscode/settings.json
9efa501 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
b4d5c24 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
fcfcd62 
Co-authored-by: David Leal <halfpacho@gmail.com>
@ayush523 @Panquesito7
Update dynamic_programming/kadane2.cpp
d3b4669 
Co-authored-by: David Leal <halfpacho@gmail.com>
Panquesito7
Panquesito7 approved these changes Sep 20, 2020
View reviewed changes
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, good work. 👍 😄

@Panquesito7 Panquesito7 added approved Approved; waiting for merge and removed requested changes changes have been requested labels Sep 20, 2020
@Panquesito7 Panquesito7 requested a review from kvedala September 20, 2020 17:16
@ayush523
Copy link
Contributor Author

ayush523 commented Sep 20, 2020

Thank you @Panquesito7

@kvedala
Copy link
Collaborator

kvedala commented Sep 23, 2020

@Panquesito7 do you confirm the algorithm is a different implementation than the existing one? thank you

@Panquesito7
Copy link
Member

Panquesito7 commented Sep 23, 2020

Yes, implementation looks different. 🙂
I like this one rather than the current one:

  • Has very good documentation
  • Uses C++ standard-libraries/functions
  • Well-structured and as per the repository standards

Can be merged. 👍

@kvedala kvedala merged commit 7f7b2b0 into TheAlgorithms:master Sep 23, 2020
@ayush523 ayush523 deleted the kadane branch September 24, 2020 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Panquesito7 Panquesito7 Panquesito7 approved these changes

@kvedala kvedala Awaiting requested review from kvedala

Assignees

No one assigned

Labels

approved Approved; waiting for merge enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEATURE]Create Kadane's Algorithm

3 participants

@ayush523 @Panquesito7 @kvedala