NITSkmOS / Algorithms

In case you want to contribute, ping on https://gitter.im/NITSkmOS/algo.
https://nitskmos.github.io/Algorithms
MIT License
99 stars 302 forks source link

add binary search in c #593

Closed varunkmr19 closed 3 years ago

varunkmr19 commented 3 years ago

For short term contributors: we understand that getting your commits well defined like we require is a hard task and takes some learning. If you look to help without wanting to contribute long term there's no need for you to learn this. Just drop us a message and we'll take care of brushing up your stuff for merge!

Fixes #44

By submitting this pull request I confirm I've read and complied with the below declarations.

After you submit your pull request, DO NOT click the 'Update Branch' button.

varunkmr19 commented 3 years ago

I went through the logs of Travis CI. It says there are some extra whitespaces in my source code. So, should I fix it or is it good enough for a merge?

sangamcse commented 3 years ago

Yes @varunkmr19, the fix is needed. Also, it would be great if you go through https://github.com/NITSkmOS/Algorithms/blob/master/CONTRIBUTING.md#write-good-commit-messages and do the necessary changes. Also, can you add entry in README.md?

sameerchoubey commented 3 years ago

@varunkmr19 There are few things that need to be sorted for this PR to get merged.

a) At line 5, we cannot obtain the size of an array after passing it inside a function because only the first address of the array is passed inside the function. When we pass an array to a function, it decays into a pointer to the first element, at which point knowledge of its size is not known. So, it would be something like this, after the change, int main(){ int end = sizeof(arr)/sizeof(arr[0]); int ans = binary_search(arr, end, target); }

b) Also, update the README file and remove the extra spacing.

Everything else looks good.

varunkmr19 commented 3 years ago

Don't know why It's not passing all the checks. Even after I fixed the code and removed the extra spacing as well.

varunkmr19 commented 3 years ago

Finally, I found out what the issue was. I had installed prettier(a VScode extension) which was messing with the alignment.

sameerchoubey commented 3 years ago

That's great. But, please squash your commits.

sangamcse commented 3 years ago

@varunkmr19 can you please go through the review comments and do the changes that are requested?