docs: add comments and explanation to binary_search_tree.cpp data_structures #891

liushubin-gitHub · 2020-06-24T08:06:40Z

Description of Change

This pull request make the following changes:

docs: add comments and explanation to binary_search_tree.cpp data_structures
test: add test for binary_search_tree.cpp data_structures
fix: Remove() function bugs in binary_search_tree.cpp data_structures

Checklist

Added description of change
[x ] PR title follows semantic commit guidelines
[x ] Search previous suggestions before making a new one, as yours may be a duplicate.
[x ] I acknowledge that all my contributions will be made under the project's license.

@kvedala Pls, help review!
Notes:

…uctures test: add test for binary_search_tree.cpp data_structures fix: Remove() function bugs in binary_search_tree.cpp data_structures

data_structures/binary_search_tree.cpp

use assert() for test

lgtm-com · 2020-06-25T14:38:44Z

This pull request introduces 1 alert when merging 6b10a56 into 8cd25f4 - view on LGTM.com

new alerts:

1 for Short global name

kvedala · 2020-06-25T14:40:19Z

rule of thumb - never use global variables unless absolutely necessary

liushubin-gitHub · 2020-06-25T14:53:36Z

I will find another way to make it better.

delete globle variable.

liushubin-gitHub · 2020-06-28T07:23:49Z

fix lgtm alerts: remove global variables.

kvedala · 2020-07-03T10:11:30Z

@liushubin-gitHub any update on this with fixing the errors? Thank you

liushubin-gitHub · 2020-07-03T10:29:16Z

yes it is done .
lgtm has passed

kvedala · 2020-07-03T11:31:06Z

yes it is done .
lgtm has passed

Thank you.
There are still undocumented structures and functions. Can you please document them as well.

liushubin-gitHub · 2020-07-03T11:46:10Z

yes it is done .
lgtm has passed

Thank you.
There are still undocumented structures and functions. Can you please document them as well.

yes，I will try my best to do it.
But there is still some functions that i don't understand clearly,so i just leave it empty. (as i am not the original author)
Also, there should be some document about test cases , but i think not so much important (test code is clearly enough), then i just skip :)

kvedala · 2020-07-03T11:51:01Z

yes it is done .
lgtm has passed

Thank you.
There are still undocumented structures and functions. Can you please document them as well.

yes，I will try my best to do it.
But there is still some functions that i don't understand clearly,so i just leave it empty. (as i am not the original author)
Also, there should be some document about test cases , but i think not so much important (test code is clearly enough), then i just skip :)

Yes, there are some codes that are not properly documented. It is an ongoing effort to fic them: see for example #554 #555
Leaving code half-fixed will lead to issues in maintaining them as they will be inconsistent and contributors can have the wrong idea. Moreover, the repo auto-generates a documentation website - https://thealgorithms.github.io/C that will not render correctly if code is only half fixed.

liushubin-gitHub · 2020-07-05T14:18:49Z

yes it is done .
lgtm has passed

Thank you.
There are still undocumented structures and functions. Can you please document them as well.

yes，I will try my best to do it.
But there is still some functions that i don't understand clearly,so i just leave it empty. (as i am not the original author)
Also, there should be some document about test cases , but i think not so much important (test code is clearly enough), then i just skip :)

Yes, there are some codes that are not properly documented. It is an ongoing effort to fic them: see for example #554 #555
Leaving code half-fixed will lead to issues in maintaining them as they will be inconsistent and contributors can have the wrong idea. Moreover, the repo auto-generates a documentation website - https://thealgorithms.github.io/C that will not render correctly if code is only half fixed.

Got it，i will try it again later

liushubin-gitHub · 2020-07-10T11:43:37Z

Pls review!

kvedala · 2020-07-10T12:13:41Z

Please fix the errors reported by the system. You can click on "details" to see the error details

kvedala

Please fix the dynamic memory and the errors reported by the automated tests

data_structures/binary_search_tree.cpp

kvedala · 2020-07-10T13:49:35Z

please check the errors reported

liushubin-gitHub · 2020-07-10T17:59:00Z

please check the errors reported

i found the code format fail has been auto corrected by the code formatter!

kvedala · 2020-07-10T18:00:49Z

please check the errors reported

i found the code format fail has been auto corrected by the code formatter!

I don't think so. Can you show me where you see that?
Did you actually click the details link and see the error message?

liushubin-gitHub · 2020-07-15T13:47:11Z

Pls help review!

kvedala

👍
@ayaankhan98 please review as well. thanks.

kvedala · 2020-07-15T16:35:07Z

could you add that in the std::out statements that any other number will exit

ayaankhan98 · 2020-07-15T16:59:40Z

data_structures/binary_search_tree.cpp

+ * \param[in] x a node with value to be insert
+ */
+void Insert(node* n, int x) {
+    assert(n != NULL);


what happens if the user inserts the very first root element? this Insert function will break in that case. you are not handling the root insertition case, instead you assumed to insert the root value manually as mentioned in the test function

root->val = 4; root->left = nullptr; root->right = nullptr;

i suggest you to handle this too in the insert function because basically we are performing the very same operation that is we are insterting a value let it be anywhere in the tree either the root or the child everything should be properly handled by the same insert function

@ayaankhan98 Thanks for you suggestion. I have got your means:
you want to design a function that is very common to use.
But , since here we are talking about a special tree (binary search tree).We need to insert a node from root node but can't from any children node (like a normal tree).
Another problem: it just assert and break the program when user take a wrong use of Insert() with a root node to be NULL .
I think with a kind use prompt for user .Maybe better than exit program.

https://github.com/ayaankhan98/DSA_competative/blob/b1d200d8a9614141fa1305f1e41c605526e91336/Data_Structures/Trees/BST.cpp#L25

have a look at this code that which i had written a year ago, i hope this makes you clear what i wish to say.

if (root == nullptr) { Node* new_node = new Node; new_node->data = data; new_node->left = nullptr; new_node->right = nullptr; root = new_node; }

if the root is null then we can do something like this. @liushubin-gitHub And i think this would be better instead of inserting the root node manually like

root->val = 4; root->left = nullptr; root->right = nullptr;

Got it.Thanks a lot!

Nice look at that link i had implemented some extra methods on BST, see if you can implement that in this PR.
@liushubin-gitHub if you face any problem let me know.

liushubin-gitHub · 2020-07-16T00:52:31Z

any other number will exit

done

data_structures/binary_search_tree.cpp

kvedala · 2020-07-16T20:15:19Z

@liushubin-gitHub It seems your latest push broke the code. Please revert to the last known good code.

liushubin-gitHub

revert to last good code

revert to last good code。 This reverts commit adb42e4.

liushubin-gitHub · 2020-07-17T01:03:41Z

@liushubin-gitHub It seems your latest push broke the code. Please revert to the last known good code.

OK，done。

revert to last good code This reverts commit 19bdd29.

ayaankhan98 · 2020-07-18T15:33:05Z

data_structures/binary_search_tree.cpp

+ * \param[in] x a node with value to be insert
+ */
+void Insert(node* root, int x) {
+    if (root == NULL) {


please consider the changes for the insert function which i suggested earlier.

yes.i try early.but auto format checker failed.
you can get in commit:
adb42e4

so.i revert to last good code.

@liushubin-gitHub what was the actual error? is removing the code is only the solution?

if (root == nullptr) { root = std::shared_ptr<node>(new node); root->val = x; root->left = nullptr; root->right = nullptr; return; }

liushubin-gitHub · 2020-07-23T16:05:51Z

could non_const reference be marked a warning instead of an error
Pls review!

stale · 2021-07-01T03:07:14Z

This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2021-08-28T17:37:49Z

This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2021-09-06T18:56:23Z

Please ping one of the maintainers once you commit the changes requested or make improvements on the code. If this is not the case and you need some help, feel free to ask for help in our Gitter channel. Thank you for your contributions!

liushubin lwx470335 and others added 3 commits June 24, 2020 14:25

docs: add comments and explanation to binary_search_tree.cpp data_str…

a702702

…uctures test: add test for binary_search_tree.cpp data_structures fix: Remove() function bugs in binary_search_tree.cpp data_structures

formatting source-code for a702702

7b53dda

Merge branch 'master' into main

8d4bcc8

liushubin-gitHub mentioned this pull request Jun 24, 2020

fix: Insert() does not work issue in binary_search_tree.cpp #882

Merged

1 task

kvedala reviewed Jun 24, 2020

View reviewed changes

data_structures/binary_search_tree.cpp Show resolved Hide resolved

liushubin-gitHub and others added 3 commits June 25, 2020 22:20

test: add test for binary_search_tree.cpp data_structures

f377860

use assert() for test

formatting source-code for f377860

3e8b19f

Update binary_search_tree.cpp

6b10a56

liushubin lwx470335 and others added 2 commits June 28, 2020 15:06

test: add test for binary_search_tree.cpp data_structures

dba2daf

delete globle variable.

formatting source-code for dba2daf

6910ca5

liushubin lwx470335 and others added 2 commits July 10, 2020 17:12

fix: modify BFT() breath first search a tree

4f62e91

formatting source-code for 4f62e91

49ccaa6

kvedala requested changes Jul 10, 2020

View reviewed changes

data_structures/binary_search_tree.cpp Outdated Show resolved Hide resolved

liushubin-gitHub and others added 2 commits July 10, 2020 21:30

fix:free memory for auto test case

cf8917e

formatting source-code for cf8917e

9dafdfa

kvedala previously approved these changes Jul 15, 2020

View reviewed changes

ayaankhan98 reviewed Jul 15, 2020

View reviewed changes

liushubin lwx470335 and others added 2 commits July 16, 2020 09:04

prompt message optimize.

d51dfff

formatting source-code for d51dfff

fb5971d

ayaankhan98 reviewed Jul 16, 2020

View reviewed changes

data_structures/binary_search_tree.cpp Show resolved Hide resolved

liushubin-gitHub and others added 2 commits July 16, 2020 23:17

optimize Insert() function as review suggestion.

adb42e4

formatting source-code for adb42e4

2e13fd6

liushubin-gitHub commented Jul 17, 2020

View reviewed changes

Revert "optimize Insert() function as review suggestion."

051bc1b

revert to last good code。 This reverts commit adb42e4.

liushubin-gitHub and others added 3 commits July 17, 2020 20:40

optimize Insert() Remove() function to use const reference.

19bdd29

formatting source-code for 19bdd29

0d844ff

Revert "optimize Insert() Remove() function to use const reference."

0932f88

revert to last good code This reverts commit 19bdd29.

kvedala requested a review from ayaankhan98 July 17, 2020 23:05

ayaankhan98 reviewed Jul 18, 2020

View reviewed changes

kvedala requested a review from ayaankhan98 July 29, 2020 17:49

github-actions bot force-pushed the master branch from eca078d to a6a3b7d Compare October 2, 2020 12:23

stale bot added the stale Author has not responded to the comments for over 2 weeks label Jul 1, 2021

Panquesito7 dismissed kvedala’s stale review via 0932f88 July 29, 2021 16:42

stale bot removed the stale Author has not responded to the comments for over 2 weeks label Jul 29, 2021

stale bot added the stale Author has not responded to the comments for over 2 weeks label Aug 28, 2021

stale bot closed this Sep 6, 2021

docs: add comments and explanation to binary_search_tree.cpp data_structures #891

docs: add comments and explanation to binary_search_tree.cpp data_structures #891

Conversation

liushubin-gitHub commented Jun 24, 2020 • edited by gitpod-io bot

Description of Change

Checklist

lgtm-com bot commented Jun 25, 2020

kvedala commented Jun 25, 2020 • edited

liushubin-gitHub commented Jun 25, 2020

liushubin-gitHub commented Jun 28, 2020

kvedala commented Jul 3, 2020

liushubin-gitHub commented Jul 3, 2020

kvedala commented Jul 3, 2020

liushubin-gitHub commented Jul 3, 2020

kvedala commented Jul 3, 2020 • edited

liushubin-gitHub commented Jul 5, 2020

liushubin-gitHub commented Jul 10, 2020

kvedala commented Jul 10, 2020

kvedala left a comment

Choose a reason for hiding this comment

kvedala commented Jul 10, 2020

liushubin-gitHub commented Jul 10, 2020

kvedala commented Jul 10, 2020

liushubin-gitHub commented Jul 15, 2020

kvedala left a comment

Choose a reason for hiding this comment

kvedala commented Jul 15, 2020 • edited

ayaankhan98 Jul 15, 2020 • edited

Choose a reason for hiding this comment

liushubin-gitHub Jul 16, 2020 • edited

Choose a reason for hiding this comment

ayaankhan98 Jul 16, 2020

Choose a reason for hiding this comment

liushubin-gitHub Jul 16, 2020

Choose a reason for hiding this comment

ayaankhan98 Jul 16, 2020

Choose a reason for hiding this comment

liushubin-gitHub commented Jul 16, 2020

kvedala commented Jul 16, 2020

liushubin-gitHub left a comment

Choose a reason for hiding this comment

liushubin-gitHub commented Jul 17, 2020

ayaankhan98 Jul 18, 2020

Choose a reason for hiding this comment

liushubin-gitHub Jul 19, 2020

Choose a reason for hiding this comment

ayaankhan98 Jul 19, 2020 • edited

Choose a reason for hiding this comment

ayaankhan98 Jul 30, 2020 • edited

Choose a reason for hiding this comment

liushubin-gitHub commented Jul 23, 2020

stale bot commented Jul 1, 2021

stale bot commented Aug 28, 2021

stale bot commented Sep 6, 2021

liushubin-gitHub commented Jun 24, 2020 •

edited by gitpod-io bot

kvedala commented Jun 25, 2020 •

edited

kvedala commented Jul 3, 2020 •

edited

kvedala commented Jul 15, 2020 •

edited

ayaankhan98 Jul 15, 2020 •

edited

liushubin-gitHub Jul 16, 2020 •

edited

ayaankhan98 Jul 19, 2020 •

edited

ayaankhan98 Jul 30, 2020 •

edited