-
Notifications
You must be signed in to change notification settings - Fork 5k
Insersion sort: rename variables with convenience names #887
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
Conversation
richard-ash
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool thanks for the update 😄
Couple small stylistic comments otherwise looks good!
| while y > 0 && a[y] < a[y - 1] { // 3 | ||
| a.swapAt(y - 1, y) | ||
| y -= 1 | ||
| var sortedArray = array // 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep the // 1, // 2, etc vertical with each other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you clarify what you mean by making them vertical
| var currentIndex = index | ||
| let temp = sortedArray[currentIndex] | ||
| while currentIndex > 0 && temp < sortedArray[currentIndex - 1] { | ||
| sortedArray[currentIndex] = sortedArray[currentIndex - 1] // 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
|
Thanks for the updates! The last two comments still need updating then I'll merge 🙂 |
|
Hi Richard, I will complete it today, thanks
…On Fri, 6 Sep 2019 at 07:36, Richard Ash ***@***.***> wrote:
Thanks for the updates! The last two comments still need updating then
I'll merge 🙂
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#887>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABK4BWV33I3IY3SH4DIW24LQIHT55ANCNFSM4IL5YGAQ>
.
--
<https://about.me/abuzeidibrahim?promo=email_sig&utm_source=product&utm_medium=email_sig&utm_campaign=gmail_api&utm_content=thumb>
Abuzeid Ibrahim
<https://about.me/abuzeidibrahim?promo=email_sig&utm_source=product&utm_medium=email_sig&utm_campaign=gmail_api&utm_content=thumb>Software
Engineer
Mobile: +2 01061933229
linkedin.com/in/abuzeid-ibrahim
github.com/abuzeid-ibrahim
about.me/abuzeidibrahim
<https://about.me/abuzeidibrahim?promo=email_sig&utm_source=product&utm_medium=email_sig&utm_campaign=gmail_api&utm_content=thumb>
|
|
Hi, I don't understand why the Pr still open till now :( |
Checklist
Description