JavaScript icon indicating copy to clipboard operation
JavaScript copied to clipboard

Added Bitonic Sort Algorithm

Open prasad-chavan1 opened this issue 1 year ago • 3 comments

Open in Gitpod  know more

Describe your change:

  • [x] Add an algorithm?
  • [ ] Fix a bug or typo in an existing algorithm?
  • [ ] Documentation change?

Checklist:

  • [x] I have read CONTRIBUTING.md.
  • [x] This pull request is all my own work -- I have not plagiarized.
  • [x] 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.
  • [x] All new JavaScript files are placed inside an existing directory.
  • [x] 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
  • [x] All new algorithms have a URL in their comments that points to Wikipedia or another similar explanation.
  • [x] If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

prasad-chavan1 avatar Sep 24 '23 03:09 prasad-chavan1

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.

prasad-chavan1 avatar Sep 24 '23 03:09 prasad-chavan1

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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163365 :

@@ -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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163419 :

  • */ +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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163475 :

  • */ +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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163488 :

+}

+// 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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163540 :

  • // 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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163551 :

  • // 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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163560 :

+// 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 https://github.com/TheAlgorithms/JavaScript/pull/1370#discussion_r1335163601 :

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

This should use proper Jest tests.

— Reply to this email directly, view it on GitHub https://github.com/TheAlgorithms/JavaScript/pull/1370#pullrequestreview-1641038150, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXFPHSQARWGOBP7473RJ6KTX4AI3RANCNFSM6AAAAAA5EULERY . You are receiving this because you authored the thread.Message ID: @.***>

prasad-chavan1 avatar Sep 24 '23 12:09 prasad-chavan1

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..

prasad-chavan1 avatar Sep 26 '23 02:09 prasad-chavan1