Added Quick Select [C] #53
Conversation
|
@yashLadha: Please mention an issue that is completely closed by this PR. |
|
Also it is clear that you have made a false tick on the requirements mentioned for the PR. Please correct the errors. |
|
@yashLadha : This PR is not consistent with contributing guidelines. Please refer to it and make the changes. |
|
The closing issue link is mentioned in my commit message. |
quick_select/quick_select.c
Outdated
| } else { | ||
| return quick_select(ar, pivot_index + 1, right, pos); | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
Though this case will never be reached, it looks confusing to return 0 when we have to return one of the numbers from the array. What do you think @yashLadha?
There was a problem hiding this comment.
Sir it is because of a warning that all the return statements are inside the conditional blocks and there is a warning issued at this point because function will not return any value if in any hypothetical case it doesn't go into any conditional block statement, and it requires a return statement. If you wan't i can remove it?
There was a problem hiding this comment.
I don't think that there would be a warning.
quick_select/quick_select.c
Outdated
| } | ||
|
|
||
| int quick_select(int *ar, int left, int right, int pos) { | ||
| int pivot_index = 0; |
There was a problem hiding this comment.
Assigned value to variable pivot_index has not been used.
| int ar[] = {10, 5, 1, 6, 7, 3, 2, 4, 8, 9}; | ||
| printf("%d\n", quick_select(ar, 0, 9, 3)); | ||
| return 0; | ||
| } |
quick_select/quick_select.c
Outdated
| @@ -0,0 +1,45 @@ | |||
| #include <stdio.h> | |||
| #include <stdlib.h> | |||
yashLadha
force-pushed
the
select_algo
branch
4 times, most recently
from
3bbc5ce to
b1fb210
Compare
|
Ammended Necessary changes. |
|
Looks okay. Let's wait for some more eyes. |
| @@ -0,0 +1,45 @@ | |||
| #include <stdio.h> | |||
|
|
|||
There was a problem hiding this comment.
It would be nice to add comments about the parameters making it easy for a reader to know about the work of the parameters.
There was a problem hiding this comment.
added the comments. Can review them.
quick_select/quick_select.c
Outdated
| * right : right index of the subarray | ||
| * pos : position to find the element using quick sort | ||
| * pivot_index : pivot index | ||
| * return : int |
There was a problem hiding this comment.
Better be specific about what int.
There was a problem hiding this comment.
It is the return type of function
There was a problem hiding this comment.
But it looks strange. We have a list of parameters and out of nowhere we see the return type. What I see is that the return type is already indicated very clearly in C (int quick_select(...) {}). It would be better to add what it returns instead of return type.
There was a problem hiding this comment.
the logical meaning or the variable name.
Closes #10
|
@Monal5031 : Do things look good? |
Fixes #10
By submitting this pull request I confirm I've read and complied with the below requirements.
Added {Algorithm name} [{Language}], notUpdate README.mdorAdded new code.