feat: add Pigeonhole algorithm #1028

Lownish · 2020-08-15T05:09:04Z

Description of Change

Checklist

Added description of change
Added file name matches File name guidelines
Added tests and example, test must pass
Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
Relevant documentation/comments is changed or added
PR title follows semantic commit guidelines
Search previous suggestions before making a new one, as yours may be a duplicate.
I acknowledge that all my contributions will be made under the project's license.

Notes:

Lownish · 2020-08-15T05:25:47Z

@Panquesito7 , can you please let me know what are the actual issues with the code formatter? I defined everything as per the contribution guidelines.

rishabh-997 · 2020-08-15T07:40:30Z

what are the actual issues with the code formatter?

You are using c-style arrays for initialization. Either use a pointer or replace it with vector

Panquesito7

Please fix clang-tidy warnings.

Lownish · 2020-08-16T16:49:40Z

Please fix clang-tidy warnings.

The warning is telling to use std::vector on line 34, but it's already std::vector

sorting/pigeonhole_sort.cpp

Panquesito7 · 2020-08-16T17:00:33Z

but it's already std::vector

It's still wrong, it's a C VLA array, which is not good.
Please use std::vector<> properly.

kvedala · 2020-08-17T15:11:10Z

The repo CI tools build live documentations using doxygen and hence require a certain standard and protocol to be followed.
Please use the code structure similar to the other codes in the folder like this.
Thank you

Panquesito7

Looks much better. Good work! 👍
Now, let's add a namespace, to ensure all functions are together.

Let us also improve the code to make refined. 🙂

sorting/pigeonhole_sort.cpp

Panquesito7 · 2020-08-17T19:28:06Z

sorting/pigeonhole_sort.cpp

+#include <iostream> //for io operations
+
+/**
+ *Pigeonhole sorting of array with array size n


Missing parameter and return statement documentation.

Suggested changes applied Co-authored-by: David Leal <halfpacho@gmail.com>

sorting/pigeonhole_sort.cpp

Co-authored-by: David Leal <halfpacho@gmail.com>

sorting/pigeonhole_sort.cpp

Co-authored-by: David Leal <halfpacho@gmail.com>

sorting/pigeonhole_sort.cpp

Co-authored-by: David Leal <halfpacho@gmail.com>

Panquesito7 · 2020-08-19T18:47:11Z

sorting/pigeonhole_sort.cpp

+/**
+ * Pigeonhole sorting of array of size n
+ * The function will sort the array through Pigeonhole algorithm and print
+ * @param arr unsorted array of elements


Suggested change

* @param arr unsorted array of elements

* @param arr unsorted array of elements

* @param n number of times loop will run

n is not needed. isn't it same as the template variable N?
"Number of times loop will run" -> isn't this the number of values in your array?

sorting/pigeonhole_sort.cpp

Co-authored-by: David Leal <halfpacho@gmail.com>

Panquesito7

Excellent work; LGTM. 👍😄
Thank you for your contribution.

@kvedala please review as well. Thanks.

kvedala

there seem to be some problems in the code. please check the comments. Thank you

sorting/pigeonhole_sort.cpp

kvedala · 2020-08-23T14:43:18Z

sorting/pigeonhole_sort.cpp

+/**
+ * Pigeonhole sorting of array of size n
+ * The function will sort the array through Pigeonhole algorithm and print
+ * @param arr unsorted array of elements


n is not needed. isn't it same as the template variable N?
"Number of times loop will run" -> isn't this the number of values in your array?

sorting/pigeonhole_sort.cpp

kvedala · 2020-08-23T18:55:22Z

Any reason to use std::max_element rather than std::max similarly for std::min?

Lownish · 2020-08-24T07:53:30Z

Any reason to use std::max_element rather than std::max similarly for std::min?

I was getting an error involving incompatible data types. I could overcome this issue through the use of std::max_element

kvedala · 2020-08-25T12:47:35Z

Any reason to use std::max_element rather than std::max similarly for std::min?

I was getting an error involving incompatible data types. I could overcome this issue through the use of std::max_element

got it, std::max_element is for container classes like arrays.

sorting/pigeonhole_sort.cpp

kvedala

👍 please check the comments

sorting/pigeonhole_sort.cpp

Co-authored-by: Krishna Vedala <7001608+kvedala@users.noreply.github.com>

kvedala

👍

feat: add Pigeonhole algorithm

0fb2ac1

Panquesito7 added automated tests are failing Do not merge until tests pass enhancement New feature or request labels Aug 15, 2020

Executed clang-format

66d009e

Used pointers and vector

7789288

Panquesito7 requested changes Aug 16, 2020

View reviewed changes

Panquesito7 added the requested changes changes have been requested label Aug 16, 2020

Panquesito7 requested changes Aug 16, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

sorting/pigeonhole_sort.cpp Show resolved Hide resolved

Corrected clang-tidy issues

e8292d4

Modified code structure

5515dbc

Panquesito7 removed the automated tests are failing Do not merge until tests pass label Aug 17, 2020

Panquesito7 requested changes Aug 17, 2020

View reviewed changes

Lownish and others added 2 commits August 18, 2020 01:12

Apply suggestions from code review

71a790c

Suggested changes applied Co-authored-by: David Leal <halfpacho@gmail.com>

Added missing parameter and documentation

8884ddb

Panquesito7 reviewed Aug 17, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

lsurface added 2 commits August 18, 2020 19:22

Added delete function

02369ec

Merge branch 'master' of https://github.com/Lownish/C-Plus-Plus

656c03a

This comment has been minimized.

Sign in to view

Panquesito7 requested changes Aug 18, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

Lownish and others added 2 commits August 19, 2020 01:11

Update pigeonhole_sort.cpp

0b6acc1

Corrected delete function

f4ac5c3

Co-authored-by: David Leal <halfpacho@gmail.com>

Panquesito7 requested changes Aug 18, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

42d75af

Co-authored-by: David Leal <halfpacho@gmail.com>

Panquesito7 requested changes Aug 18, 2020

View reviewed changes

Apply suggestions from code review

fdf4507

Co-authored-by: David Leal <halfpacho@gmail.com>

Apply suggestions from code review

db8035b

Co-authored-by: David Leal <halfpacho@gmail.com>

Panquesito7 requested changes Aug 19, 2020

View reviewed changes

assert moved to test function

a8d66fa

Panquesito7 requested changes Aug 20, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

Update pigeonhole_sort.cpp

a002b3a

Panquesito7 reviewed Aug 20, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

Update sorting/pigeonhole_sort.cpp

e6a7743

Co-authored-by: David Leal <halfpacho@gmail.com>

Panquesito7 approved these changes Aug 20, 2020

View reviewed changes

Panquesito7 added approved Approved; waiting for merge and removed requested changes changes have been requested labels Aug 20, 2020

Panquesito7 requested a review from kvedala August 20, 2020 17:24

kvedala requested changes Aug 23, 2020

View reviewed changes

lsurface added 2 commits August 23, 2020 20:56

Added test function and inbuilt min function

188aec1

Merge branch 'master' of https://github.com/Lownish/C-Plus-Plus

51d82df

kvedala reviewed Aug 25, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

lsurface added 3 commits August 25, 2020 18:36

min and max to const variables

81efb63

const int* to auto

f711f72

const int* to auto

69167a9

Lownish requested a review from kvedala August 25, 2020 16:10

kvedala reviewed Aug 25, 2020

View reviewed changes

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

sorting/pigeonhole_sort.cpp Outdated Show resolved Hide resolved

Lownish and others added 2 commits August 25, 2020 23:10

Apply suggestions from code review

6d9920a

Co-authored-by: Krishna Vedala <7001608+kvedala@users.noreply.github.com>

Modified pigeonSort documentation

7165bb3

Panquesito7 added the automated tests are failing Do not merge until tests pass label Aug 25, 2020

Corrected test functions

4424320

Panquesito7 removed the automated tests are failing Do not merge until tests pass label Aug 25, 2020

kvedala approved these changes Aug 25, 2020

View reviewed changes

Panquesito7 merged commit 66dcc4c into TheAlgorithms:master Aug 25, 2020

	* @param arr unsorted array of elements
	* @param arr unsorted array of elements
	* @param n number of times loop will run

Uh oh!

feat: add Pigeonhole algorithm #1028

feat: add Pigeonhole algorithm #1028

Uh oh!

Conversation

Lownish commented Aug 15, 2020 • edited by gitpod-io bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

Lownish commented Aug 15, 2020

rishabh-997 commented Aug 15, 2020

Panquesito7 left a comment

Choose a reason for hiding this comment

Lownish commented Aug 16, 2020

Uh oh!

Uh oh!

Panquesito7 commented Aug 16, 2020

kvedala commented Aug 17, 2020

Panquesito7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Panquesito7 Aug 17, 2020

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Panquesito7 Aug 19, 2020

Choose a reason for hiding this comment

kvedala Aug 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Panquesito7 left a comment

Choose a reason for hiding this comment

kvedala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kvedala Aug 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kvedala commented Aug 23, 2020

Lownish commented Aug 24, 2020

kvedala commented Aug 25, 2020

Uh oh!

kvedala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kvedala left a comment

Choose a reason for hiding this comment

Labels

5 participants

Lownish commented Aug 15, 2020 •

edited by gitpod-io bot

Loading