feat: add binary insertion sort algorithm#71
Conversation
raklaptudirm
left a comment
raklaptudirm
left a comment
There was a problem hiding this comment.
We already have a binary search implementation, so use that instead of making your own.
|
Hi @raklaptudirm, the existing binary search will return -1 if we don't find the element. But my modified version find the position of the element that is to be inserted in the array. So I cant reuse it. |
|
Ah, makes sense. |
|
@appgurueu Can you please review this PR |
You could consider updating the existing binary search. |
|
@appgurueu See the actuall Binary Search should return -1 if the key is not found. But in my function, I will be returning the insertion position of the key element. |
No. Our current implementation behaves this way because it is lazily written. More advanced implementations return the insertion or occurrence index; it is then trivial to check whether the element was found ( |
|
@appgurueu yeah, correct. How to proceed mate ? Do i need to close this PR and create a new one for Binary Search or Include the fix for binary search in this PR itself. ? |
I'd be fine with including the extension/feature (not a fix, the current implementation is correct) in this PR. |
|
@appgurueu I have created a PR for Binary Search Improvement |
|
@appgurueu What about this PR? I think this can be closed |
It can be closed, yes, but why should it be closed? It would add something if it were merged. It's just that it currently depends on #72. The author should be able pick this up at any time. |
|
An sorry, I understood this wrongly. I thought this PR was canceled |
github-actions
Bot
force-pushed
the
master
branch
from
fb7b311 to
d4d9abb
Compare
appgurueu
left a comment
appgurueu
left a comment
There was a problem hiding this comment.
The binary search is still duplicated locally.
5 participants
No description provided.