Added Bitonic Sort Algorithm #1370

prasad-chavan1 · 2023-09-24T03:23:08Z

Describe your change:

Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?

Checklist:

I have read CONTRIBUTING.md.
This pull request is all my own work -- I have not plagiarized.
I know that pull requests will not be merged if they fail the automated tests.
This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
All new JavaScript files are placed inside an existing directory.
All filenames should use the UpperCamelCase (PascalCase) style. There should be no spaces in filenames.
Example:UserProfile.js is allowed but userprofile.js,Userprofile.js,user-Profile.js,userProfile.js are not
All new algorithms have a URL in their comments that points to Wikipedia or another similar explanation.
If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

prasad-chavan1 · 2023-09-24T03:47:35Z

Respected @appgurueu & @raklaptudirm ,

I apologize for any inconvenience caused by my recent pull request. It includes code indentation fixes that I'm addressing promptly. Please consider merging the request, and I'll ensure it aligns perfectly with project standards in the next update. Your guidance is appreciated.

appgurueu

Please also add a proper JSDoc comment. Try to reuse the tests of existing sorting algorithms (but don't just copy and paste; simply execute the same tests for multiple sorting algorithms that offer the same API).

appgurueu · 2023-09-24T11:11:34Z

Sorts/BitonicSort.js

+    * more information: https://en.wikipedia.org/wiki/Bitonic_sorter
+
+    */
+function compAndSwap (a, i, j, order) {


Rather than taking an order parameter, this should take a comparison function.

appgurueu · 2023-09-24T11:12:04Z

Sorts/BitonicSort.js

+// order, if order = 1
+function bitonicMergeArr (a, low, cnt, dir) {
+  if (cnt > 1) {
+    const k = parseInt(cnt / 2)


Why use parseInt instead of cnt >> 1 for floor division by 2?

appgurueu · 2023-09-24T11:12:24Z

Sorts/BitonicSort.js

+function bitonicMergeArr (a, low, cnt, dir) {
+  if (cnt > 1) {
+    const k = parseInt(cnt / 2)
+    for (let i = low; i < low + k; i++) { compAndSwap(a, i, i + k, dir) }


Why not inline compAndSwap?

appgurueu · 2023-09-24T11:12:38Z

Sorts/BitonicSort.js

+
+function bitonicSort (a, low, cnt, order) { // (arr 0 arrLen AS-Ds-order)
+  if (cnt > 1) {
+    const k = parseInt(cnt / 2)


Same here (use cnt >> 1)

appgurueu · 2023-09-24T11:13:12Z

Sorts/BitonicSort.js

+
+// Calling of bitonicSort func for sorting the entire array
+// of length N in ASCENDING order
+// here up=1 for ASCENDING & up=0 for DESCENDING


Again, I'd prefer a comparator.

appgurueu · 2023-09-24T11:13:23Z

Sorts/BitonicSort.js

+}
+
+// displaying array
+function logArray (arr) {


This function doesn't belong here. It should be removed.

appgurueu · 2023-09-24T11:13:31Z

Sorts/BitonicSort.js

+
+export { sort }
+
+// Test Case method


This doesn't belong here.

appgurueu · 2023-09-24T11:14:02Z

Sorts/test/BitonicSort.test.js

@@ -0,0 +1,36 @@
+import {sort} from '../BitonicSort'


This should use proper Jest tests.

prasad-chavan1 · 2023-09-24T12:11:39Z

Okay sir it will be ready soon as per your instructions till tomorrow ❤️Have a great evening

…

On Sun, Sep 24, 2023, 4:45 PM Lars Müller ***@***.***> wrote: ***@***.**** requested changes on this pull request. Please also add a proper JSDoc comment. Try to reuse the tests of existing sorting algorithms (but don't just copy and paste; simply execute the same tests for multiple sorting algorithms that offer the same API). ------------------------------ In Sorts/BitonicSort.js <#1370 (comment)> : > @@ -0,0 +1,74 @@ +/* + + * JS program for bitonic sort + * Bitonic Sort is a parallel sorting algorithm that works by dividing the + array into two parts, recursively sorting them, and then merging them in a + specific way. + * more information: https://en.wikipedia.org/wiki/Bitonic_sorter + + */ +function compAndSwap (a, i, j, order) { Rather than taking an order parameter, this should take a comparison function. ------------------------------ In Sorts/BitonicSort.js <#1370 (comment)> : > + + */ +function compAndSwap (a, i, j, order) { + if ((a[i] > a[j] && order === 1) || (a[i] < a[j] && order === 0)) { + // Swapping elements + const temp = a[i] + a[i] = a[j] + a[j] = temp + } +} + +// It recursively sorts a bitonic sequence in ascending +// order, if order = 1 +function bitonicMergeArr (a, low, cnt, dir) { + if (cnt > 1) { + const k = parseInt(cnt / 2) Why use parseInt instead of cnt >> 1 for floor division by 2? ------------------------------ In Sorts/BitonicSort.js <#1370 (comment)> : > + */ +function compAndSwap (a, i, j, order) { + if ((a[i] > a[j] && order === 1) || (a[i] < a[j] && order === 0)) { + // Swapping elements + const temp = a[i] + a[i] = a[j] + a[j] = temp + } +} + +// It recursively sorts a bitonic sequence in ascending +// order, if order = 1 +function bitonicMergeArr (a, low, cnt, dir) { + if (cnt > 1) { + const k = parseInt(cnt / 2) + for (let i = low; i < low + k; i++) { compAndSwap(a, i, i + k, dir) } Why not inline compAndSwap? ------------------------------ In Sorts/BitonicSort.js <#1370 (comment)> : > +} + +// It recursively sorts a bitonic sequence in ascending +// order, if order = 1 +function bitonicMergeArr (a, low, cnt, dir) { + if (cnt > 1) { + const k = parseInt(cnt / 2) + for (let i = low; i < low + k; i++) { compAndSwap(a, i, i + k, dir) } + bitonicMergeArr(a, low, k, dir) + bitonicMergeArr(a, low + k, k, dir) + } +} + +function bitonicSort (a, low, cnt, order) { // (arr 0 arrLen AS-Ds-order) + if (cnt > 1) { + const k = parseInt(cnt / 2) Same here (use cnt >> 1) ------------------------------ In Sorts/BitonicSort.js <#1370 (comment)> : > + + // sort in ascending order since order here is 1 + bitonicSort(a, low, k, 1) + + // sort in descending order since order here is 0 + bitonicSort(a, low + k, k, 0) + + // Will merge whole sequence in ascending order + // since dir=1. + bitonicMergeArr(a, low, cnt, order) + } +} + +// Calling of bitonicSort func for sorting the entire array +// of length N in ASCENDING order +// here up=1 for ASCENDING & up=0 for DESCENDING Again, I'd prefer a comparator. ------------------------------ In Sorts/BitonicSort.js <#1370 (comment)> : > + + // Will merge whole sequence in ascending order + // since dir=1. + bitonicMergeArr(a, low, cnt, order) + } +} + +// Calling of bitonicSort func for sorting the entire array +// of length N in ASCENDING order +// here up=1 for ASCENDING & up=0 for DESCENDING +function sort (a, N, up) { + bitonicSort(a, 0, N, up) +} + +// displaying array +function logArray (arr) { This function doesn't belong here. It should be removed. ------------------------------ In Sorts/BitonicSort.js <#1370 (comment)> : > + +// Calling of bitonicSort func for sorting the entire array +// of length N in ASCENDING order +// here up=1 for ASCENDING & up=0 for DESCENDING +function sort (a, N, up) { + bitonicSort(a, 0, N, up) +} + +// displaying array +function logArray (arr) { + for (let i = 0; i < arr.length; ++i) { console.log(arr[i] + ' ') } +} + +export { sort } + +// Test Case method This doesn't belong here. ------------------------------ In Sorts/test/BitonicSort.test.js <#1370 (comment)> : > @@ -0,0 +1,36 @@ +import {sort} from '../BitonicSort' This should use proper Jest tests. — Reply to this email directly, view it on GitHub <#1370 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXFPHSQARWGOBP7473RJ6KTX4AI3RANCNFSM6AAAAAA5EULERY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

appgurueu

Why did this PR just change from Bitonic Sort to Polynomial Hash?

prasad-chavan1 · 2023-09-26T02:14:45Z

Why did this PR just change from Bitonic Sort to Polynomial Hash?

Sir actually am too busy...Today is my final theory paper so i didnt made any changes to that algo but instead of that i added another hash algo with proper format..so sorry for that....i'll do it by today or tommorow..

Added Bitonic sort

05f0f49

prasad-chavan1 requested review from raklaptudirm and appgurueu as code owners September 24, 2023 03:23

Update Fixes

a10b5bd

appgurueu requested changes Sep 24, 2023

View reviewed changes

Polynomial Hash

8f57cf3

appgurueu requested changes Sep 25, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Added Bitonic Sort Algorithm #1370

Added Bitonic Sort Algorithm #1370

Uh oh!

prasad-chavan1 commented Sep 24, 2023

Uh oh!

prasad-chavan1 commented Sep 24, 2023

Uh oh!

appgurueu left a comment

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

appgurueu Sep 24, 2023

Uh oh!

prasad-chavan1 commented Sep 24, 2023 via email

Uh oh!

appgurueu left a comment

Uh oh!

prasad-chavan1 commented Sep 26, 2023

Uh oh!

Uh oh!

Uh oh!

Added Bitonic Sort Algorithm #1370

Are you sure you want to change the base?

Added Bitonic Sort Algorithm #1370

Uh oh!

Conversation

prasad-chavan1 commented Sep 24, 2023

Describe your change:

Checklist:

Uh oh!

prasad-chavan1 commented Sep 24, 2023

Respected @appgurueu & @raklaptudirm ,

Uh oh!

appgurueu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prasad-chavan1 commented Sep 24, 2023 via email

Uh oh!

appgurueu left a comment

Choose a reason for hiding this comment

Uh oh!

prasad-chavan1 commented Sep 26, 2023

Uh oh!

Uh oh!