Skip to content
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

Merge sort added more efficient #1294

Closed
wants to merge 3 commits into from
Closed

Merge sort added more efficient #1294

wants to merge 3 commits into from

Conversation

BamaCharanChhandogi
Copy link
Member

Description of Change

References

Checklist

  • Added description of the change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Relevant documentation/comments are changed or added
  • PR title follows semantic commit guidelines
  • Search previous suggestions before you make a new one, since yours may be duplicated.
  • I acknowledge that all my contributions will be under the project's license.

Notes:

@BamaCharanChhandogi
Copy link
Member Author

Hey! @alexpantyukhin can you check?

* @param [in,out] b pointer to second variable
*/
void swap(int *a, int *b)
// Function to merge two sorted subarrays into one sorted array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why have you decreased the documentation please restore the old documentation

Copy link
Member Author

@BamaCharanChhandogi BamaCharanChhandogi Aug 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the documentation of the "swap" function. because here I don't use the Swap function. so why I write documentation if it! I wrote more efficient code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood! Sorry for the confusion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood! Sorry for the confusion.

}

/** Merge sort algorithm implementation
* @param a array to sort
* @param n number of elements in the array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this style of documentation was prefered

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added these portions.

*/
void merge_sort(int *a, int n, int l, int r)
// Merge sort function
void mergeSort(int arr[], int left, int right)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we use snake_case for all things naming

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@BamaCharanChhandogi
Copy link
Member Author

I have added documentation, I think no more need.

Copy link
Collaborator

@realstealthninja realstealthninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking time in reading my suggestions here are some more!


/** Main function */
int main(void)
int main()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int main()
/**
* @brief main function
* @returns 0 on successful exit
*/
int main()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added comment before main function.

printf("Original array: ");
for (int i = 0; i < n; i++) printf("%d ", arr[i]);

merge_sort(arr, 0, n - 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make a separate function for all the testing!
I am not that well read on c but if there is a static keyword please use that as well

static void tests(){
// tests here 
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the test function.

*/
#include <assert.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these need to be documented as well

#include <header.h> /// <reason for header.h>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants