PR ready: improved code quality, removed unnecessary comments

Author: Kishan-Ved

pydatastructs/linear_data_structures/_backend/cpp/arrays/ArrayForTrees.hpp

@@ -5,8 +5,6 @@
 #include <Python.h>
 #include <structmember.h>
 #include <cstdlib>
-#include <iostream>
-#include "DynamicOneDimensionalArray.hpp" // See if this is needed
 #include "OneDimensionalArray.hpp"
 #include "DynamicArray.hpp"
 #include "../../../../utils/_backend/cpp/TreeNode.hpp"
@@ -87,37 +85,28 @@ static PyObject* ArrayForTrees___str__(ArrayForTrees *self) {
 
 static PyObject* ArrayForTrees__modify(ArrayForTrees *self,
                                                     PyObject* args) {
-    // This has been tested completely except for the if () statements in the loop mentioned below.
-    // Returning of the dictionary is also tested and it works fine
 
     if (((double)self->_num/(double)self->_size) < self->_load_factor) {
         PyObject* new_indices = PyDict_New();
 
-        // PyObject* arr_new = OneDimensionalArray___new__(&TreeNodeType, reinterpret_cast<PyObject*>(2*self->_num + 1));
-        // This is how arr_new was made in DynamicOneDimensionalArray__modify() for the previous line :-
         long new_size = 2 * self->_num + 1;
         PyObject** arr_new = reinterpret_cast<PyObject**>(std::malloc(new_size * sizeof(PyObject*)));
         for( int i = 0; i < new_size; i++ ) {
             Py_INCREF(Py_None);
             arr_new[i] = Py_None;
         }
 
-        // For some debugging:
-        // return __str__(arr_new, new_size); // Prints: ['', '', '']
-
         int j=0;
-        PyObject** _data = self->_one_dimensional_array->_data; // Check this line
+        PyObject** _data = self->_one_dimensional_array->_data;
         for(int i=0; i<=self->_last_pos_filled;i++) {
-            if (_data[i] != Py_None) { // Check this line. Python code: if self[i] is not None:
-                Py_INCREF(Py_None); // This was put in DynamicOneDimensionalArray line 116
+            if (_data[i] != Py_None) {
+                Py_INCREF(Py_None);
                 arr_new[j] = _data[i];
                 PyObject_SetItem(new_indices, reinterpret_cast<TreeNode*>(_data[i])->key, PyLong_FromLong(j));
                 j += 1;
             }
         }
 
-        // The following loop has if () statements which need to be tested
-
         for(int i=0;i<j;i++) {
             if (reinterpret_cast<TreeNode*>(arr_new[i])->left != Py_None) {
                 reinterpret_cast<TreeNode*>(arr_new[i])->left = PyObject_GetItem(

pydatastructs/linear_data_structures/_backend/cpp/arrays/DynamicOneDimensionalArray.hpp

@@ -5,7 +5,6 @@
 #include <Python.h>
 #include <structmember.h>
 #include <cstdlib>
-// #include <iostream>
 #include "DynamicArray.hpp"
 #include "OneDimensionalArray.hpp"
 #include "../../../../utils/_backend/cpp/utils.hpp"

pydatastructs/linear_data_structures/_backend/cpp/arrays/OneDimensionalArray.hpp

@@ -4,7 +4,6 @@
 #define PY_SSIZE_T_CLEAN
 #include <Python.h>
 #include <structmember.h>
-// #include <iostream>
 #include <cstdlib>
 #include "Array.hpp"
 #include "../../../../utils/_backend/cpp/utils.hpp"

pydatastructs/linear_data_structures/arrays.py

@@ -407,7 +407,7 @@ def append(self, el):
             self[self._last_pos_filled + 1] = el
         self._last_pos_filled += 1
         self._num += 1
-        self._modify() #if self is ArrayForTrees, then that _modify() is called
+        self._modify()
 
     def delete(self, idx):
         if idx <= self._last_pos_filled and idx >= 0 and \
@@ -416,7 +416,7 @@ def delete(self, idx):
             self._num -= 1
             if self._last_pos_filled == idx:
                 self._last_pos_filled -= 1
-                # self._size -= 1 # Size of array
+                # self._size -= 1 # Check if size of array should be changed
             return self._modify()
 
     @property

pydatastructs/linear_data_structures/tests/test_arrays.py

@@ -134,6 +134,8 @@ def test_DynamicOneDimensionalArray2():
     A = DODA(TreeNode, [root])
     assert str(A[0]) == "(None, 1, 100, None)"
 
+# To Do: Fix this size issue:
+
 # def test_DynamicOneDimensionalArray3():
 #     DODA = DynamicOneDimensionalArray
 #     A = DODA(int, 1)
@@ -147,10 +149,6 @@ def test_DynamicOneDimensionalArray2():
 #     print(str(A))
 #     print(A.size)
 
-# test_DynamicOneDimensionalArray()
-# test_DynamicOneDimensionalArray2()
-# test_DynamicOneDimensionalArray3()
-
 def test_ArrayForTrees():
     AFT = ArrayForTrees
     root = TreeNode(1, 100)
@@ -170,6 +168,3 @@ def test_cpp_ArrayForTrees():
     node = TreeNode(2, 200, backend=Backend.CPP)
     A.append(node)
     assert str(A) == "['(None, 1, 100, None)', '(None, 2, 200, None)']"
-
-# test_ArrayForTrees()
-# test_cpp_ArrayForTrees()

pydatastructs/trees/_backend/cpp/BinarySearchTree.hpp

@@ -4,7 +4,6 @@
 #define PY_SSIZE_T_CLEAN
 #include <Python.h>
 #include <structmember.h>
-#include <iostream>
 #include <cstdlib>
 #include <stack>
 #include "../../../utils/_backend/cpp/utils.hpp"
@@ -68,7 +67,7 @@ static PyObject* BinarySearchTree__update_size(BinarySearchTree* self, PyObject
         while (walk!=Py_None) {
             TreeNode* node = reinterpret_cast<TreeNode*>(self->binary_tree->tree->_one_dimensional_array->_data[PyLong_AsLong(walk)]);
             node->size = BinarySearchTree_left_size(self, node) + BinarySearchTree_right_size(self, node) + 1;
-            walk = node->parent; // Parent is a long or a Py_None, hence, it is a PyObject
+            walk = node->parent;
         }
     }
     Py_RETURN_NONE;
@@ -84,7 +83,7 @@ static PyObject* BinarySearchTree_search(BinarySearchTree* self, PyObject* args,
     }
     BinaryTree* bt = self->binary_tree;
     PyObject* parent = Py_None;
-    PyObject* walk = PyLong_FromLong(PyLong_AsLong(bt->root_idx)); // root_idx is size_t, change it to long if needed
+    PyObject* walk = PyLong_FromLong(PyLong_AsLong(bt->root_idx));
 
     if (reinterpret_cast<TreeNode*>(bt->tree->_one_dimensional_array->_data[PyLong_AsLong(walk)])->key == Py_None) {
         Py_RETURN_NONE;
@@ -98,7 +97,6 @@ static PyObject* BinarySearchTree_search(BinarySearchTree* self, PyObject* args,
         }
         parent = walk;
 
-        // The comparator has been tested. It works fine. :)
         if (!PyCallable_Check(bt->comparator)) {
             PyErr_SetString(PyExc_ValueError, "comparator should be callable");
             return NULL;
@@ -242,7 +240,6 @@ static PyObject* BinarySearchTree_delete(BinarySearchTree* self, PyObject *args,
             PyObject* par_key = reinterpret_cast<TreeNode*>(bt->tree->_one_dimensional_array->_data[PyLong_AsLong(parent)])->key;
             PyObject* root_key = reinterpret_cast<TreeNode*>(bt->tree->_one_dimensional_array->_data[PyLong_AsLong(bt->root_idx)])->key;
             PyObject* new_indices = ArrayForTrees_delete(bt->tree, Py_BuildValue("(O)",walk));
-            // bt->size = bt->size - 1; // TO DO: Fix b.insert(12,12), b.delete(12), b.insert(12)
             if (new_indices != Py_None) {
                 a = PyDict_GetItem(new_indices, par_key);
                 bt->root_idx = PyDict_GetItem(new_indices, root_key);
@@ -656,7 +653,7 @@ static PyObject* BinarySearchTree_select(BinarySearchTree* self, PyObject* args)
             i = i - (l + 1);
         }
     }
-    Py_RETURN_NONE; // This is a dummy return
+    Py_RETURN_NONE; // dummy return statement, never executed
 }
 
 static struct PyMethodDef BinarySearchTree_PyMethodDef[] = {

pydatastructs/trees/_backend/cpp/BinaryTree.hpp

@@ -3,7 +3,6 @@
 
 #define PY_SSIZE_T_CLEAN
 #include <Python.h>
-// #include <iostream>
 #include <structmember.h>
 #include <cstdlib>
 #include "../../../utils/_backend/cpp/utils.hpp"
@@ -28,9 +27,6 @@ static PyObject* BinaryTree___new__(PyTypeObject* type, PyObject *args, PyObject
     BinaryTree *self;
     self = reinterpret_cast<BinaryTree*>(type->tp_alloc(type, 0));
 
-    // // Check what this is: (python code below:)
-    // // obj = object.__new__(cls)
-
     // Assume that arguments are in the order below. Modify the python code such that this is true (ie; pass None for other arguments)
     PyObject *key = PyObject_GetItem(args, PyZero);
     PyObject *root_data = PyObject_GetItem(args, PyOne);
@@ -41,7 +37,7 @@ static PyObject* BinaryTree___new__(PyTypeObject* type, PyObject *args, PyObject
         return NULL;
     }
     Py_INCREF(Py_None);
-    key = root_data == Py_None ? Py_None : key; // This key is the argument, not self->key
+    key = root_data == Py_None ? Py_None : key;
 
     if (PyType_Ready(&TreeNodeType) < 0) { // This has to be present to finalize a type object. This should be called on all type objects to finish their initialization.
         return NULL;
@@ -61,10 +57,6 @@ static PyObject* BinaryTree___new__(PyTypeObject* type, PyObject *args, PyObject
         return NULL;
     }
 
-    // Don't delete these 2 lines, keep these for reference:
-    // ArrayForTrees* p = reinterpret_cast<ArrayForTrees*>(PyObject_CallMethod(reinterpret_cast<PyObject*>(&ArrayForTreesType),"__new__", "OOO", &DynamicOneDimensionalArrayType, &TreeNodeType, listroot));
-    // DynamicOneDimensionalArray* p = reinterpret_cast<DynamicOneDimensionalArray*>(DynamicOneDimensionalArray___new__(&DynamicOneDimensionalArrayType, args2, kwds));
-
     Py_INCREF(Py_None);
     PyObject* args2 = Py_BuildValue("(OO)", &TreeNodeType, listroot);
     PyObject* kwds2 = Py_BuildValue("()");
@@ -82,7 +74,7 @@ static PyObject* BinaryTree___new__(PyTypeObject* type, PyObject *args, PyObject
             PyErr_SetString(PyExc_ValueError, "comparator should be callable");
             return NULL;
     }
-    self->comparator = comp; // Comparator has been tested. It works fine!
+    self->comparator = comp;
     self->is_order_statistic = PyLong_AsLong(is_order_statistic);
 
     return reinterpret_cast<PyObject*>(self);
@@ -119,7 +111,7 @@ static PyObject* BinaryTree___str__(BinaryTree *self) {
             PyList_SET_ITEM(list, i, empty_string);
         }
     }
-    return PyObject_Str(list); // use this or __str()__ (that is defined in utils)?
+    return PyObject_Str(list);
 }
 
 static struct PyMethodDef BinaryTree_PyMethodDef[] = {
@@ -129,7 +121,6 @@ static struct PyMethodDef BinaryTree_PyMethodDef[] = {
     {NULL}
 };
 
-// Check if PyMemberDef is actually needed:
 static PyMemberDef BinaryTree_PyMemberDef[] = {
     {"root_idx", T_OBJECT, offsetof(BinaryTree, root_idx), READONLY, "Index of the root node"},
     {"comparator", T_OBJECT, offsetof(BinaryTree, comparator), 0, "Comparator function"},

pydatastructs/trees/tests/test_binary_trees.py

@@ -11,13 +11,14 @@ def cust_comp(a,b):
     print("custom comparator called")
     return a<b
 
+################### C++ Backend Tests below ###################
+
 def test_cpp_BinaryTree():
-    # b = BinaryTree(1,100,comp=cust_comp,backend=Backend.CPP)
+    # b = BinaryTree(1,100,comp=cust_comp,backend=Backend.CPP) # This is how a custom comparator can be passed
     b = BinaryTree(1,100,backend=Backend.CPP)
     assert raises(NotImplementedError, b.insert) # Correctly throws NotImplementedError: This is an abstract method
     assert raises(NotImplementedError, b.delete) # Correctly throws NotImplementedError: This is an abstract method
     assert raises(NotImplementedError, b.search) # Correctly throws NotImplementedError: This is an abstract method
-    # print(str(b))
     assert str(b) == "[(None, 1, 100, None)]"
 
 # test_cpp_BinaryTree()
@@ -45,7 +46,6 @@ def test_cpp_BST1():
 def test_cpp_BST2():
     BST = BinarySearchTree
     b = BST(8, 8, backend=Backend.CPP)
-    # b = BST(8, 8)
 
     ##### insert() and delete() tests #####
     b.delete(8)
@@ -90,12 +90,10 @@ def test_cpp_BST2():
     assert str(b) ==  "[(None, 8, 8, 2), '', (None, 14, 14, None)]"
 
     bc = BST(1, 1, backend=Backend.CPP)
-    # bc = BST(1, 1)
     assert bc.insert(1, 2) is None
     assert bc.delete(1, balancing_info=True) is None
 
     b2 = BST(-8, 8, backend=Backend.CPP)
-    # b2 = BST(-8, 8)
     b2.insert(-3, 3)
     b2.insert(-10, 10)
     b2.insert(-1, 1)
@@ -105,7 +103,6 @@ def test_cpp_BST2():
     b2.insert(-14, 14)
     b2.insert(-13, 13)
     assert str(b2) ==  "[(2, -8, 8, 1), (4, -3, 3, 3), (7, -10, 10, None), (None, -1, 1, None), (6, -6, 6, 5), (None, -4, 4, None), (None, -7, 7, None), (None, -14, 14, 8), (None, -13, 13, None)]"
-    # To Do: Fix CI error in next 5 lines, if we use: assert b2.delete(-13) is True
     b2.delete(-13)
     assert str(b2) == "[(2, -8, 8, 1), (4, -3, 3, 3), (7, -10, 10, None), (None, -1, 1, None), (6, -6, 6, 5), (None, -4, 4, None), (None, -7, 7, None), (None, -14, 14, None)]"
     b2.delete(-10)
@@ -114,7 +111,6 @@ def test_cpp_BST2():
     assert str(b2) ==  "[(7, -8, 8, 1), (4, -1, 1, None), '', '', (6, -6, 6, 5), (None, -4, 4, None), (None, -7, 7, None), (None, -14, 14, None)]"
 
     bl1 = BST(backend=Backend.CPP)
-    # bl1 = BST()
     nodes = [50, 30, 90, 70, 100, 60, 80, 55, 20, 40, 15, 10, 16, 17, 18]
     for node in nodes:
         bl1.insert(node, node)
@@ -136,7 +132,6 @@ def test_cpp_BST2():
     assert raises(ValueError, lambda: bl1.lowest_common_ancestor(-3, 4, 2))
 
     bl2 = BST(backend=Backend.CPP)
-    # bl2 = BST()
     nodes = [50, 30, 90, 70, 100, 60, 80, 55, 20, 40, 15, 10, 16, 17, 18]
     for node in nodes:
         bl2.insert(node, node)
@@ -171,7 +166,6 @@ def test_cpp_BST2():
         assert bl2.select(i+1).key == select_list[i]
 
     b3 = BST(backend=Backend.CPP)
-    # b3 = BST()
     b3.insert(10, 10)
     b3.insert(18, 18)
     b3.insert(7, 7)
@@ -188,7 +182,6 @@ def test_cpp_BST2():
     assert b3.lower_bound(-1) == 7
     assert b3.lower_bound(20) is None
 
-# test_cpp_BST2()
 
 def test_cpp_BST_speed():
     BST = BinarySearchTree
@@ -206,7 +199,6 @@ def test_cpp_BST_speed():
     print("Time taken by Python backend: ",t2-t1,"s")
     print("Time taken by C++ backend:    ",t4-t3,"s")
 
-# test_cpp_BST_speed()
 
 ################### Python Tests below ###################
 
@@ -309,7 +301,6 @@ def test_BinarySearchTree():
     assert raises(ValueError, lambda: bl.lowest_common_ancestor(200, 60, 1))
     assert raises(ValueError, lambda: bl.lowest_common_ancestor(-3, 4, 1))
 
-# test_BinarySearchTree()
 
 def test_BinaryTreeTraversal():
     BST = BinarySearchTree
@@ -546,7 +537,6 @@ def test_select_rank(expected_output):
     a5.delete(2)
     test_select_rank([])
 
-# test_AVLTree()
 
 def test_BinaryIndexedTree():
 
@@ -562,7 +552,6 @@ def test_BinaryIndexedTree():
     assert t.get_sum(0, 4) == 114
     assert t.get_sum(1, 9) == 54
 
-# test_BinaryIndexedTree()
 
 def test_CartesianTree():
     tree = CartesianTree()

pydatastructs/utils/_backend/cpp/Node.hpp

@@ -15,7 +15,6 @@ static void Node_dealloc(Node *self) {
     Py_TYPE(self)->tp_free(reinterpret_cast<PyObject*>(self));
 }
 
-// Class Node has no functions, not even a __new()__ function
 
 static PyTypeObject NodeType = {
     /* tp_name */ PyVarObject_HEAD_INIT(NULL, 0) "Node",

pydatastructs/utils/_backend/cpp/TreeNode.hpp

@@ -20,21 +20,14 @@ typedef struct {
 } TreeNode;
 
 static void TreeNode_dealloc(TreeNode *self) {
-    // left and right are long values, no need to dealloc for them
-    // TreeNode_dealloc(reinterpret_cast<TreeNode*>(TreeNode->left));
-    // TreeNode_dealloc(reinterpret_cast<TreeNode*>(TreeNode->right));
-    // Check if other deallocs are needed using Py_XDECREF
     Py_TYPE(self)->tp_free(reinterpret_cast<PyObject*>(self));
 }
 
 static PyObject* TreeNode___new__(PyTypeObject* type, PyObject *args, PyObject *kwds) {
     TreeNode *self;
     self = reinterpret_cast<TreeNode*>(type->tp_alloc(type, 0));
 
-    // Check what this is: (python code below:)
-    // obj = Node.__new__(cls)
-
-    // Assume that arguments are in the order below. Modify the code such that this is true.
+    // Assume that arguments are in the order below. Python code is such that this is true.
     self->key = PyObject_GetItem(args, PyZero);
     self->data = PyObject_GetItem(args, PyOne);
 
@@ -69,6 +62,7 @@ static struct PyMemberDef TreeNode_PyMemberDef[] = {
     {NULL},
 };
 
+
 static PyTypeObject TreeNodeType = {
     /* tp_name */ PyVarObject_HEAD_INIT(NULL, 0) "TreeNode",
     /* tp_basicsize */ sizeof(TreeNode),