Data structures

Hi there, can anyone check the implementation of several data structures using what I learned from codecademy. I want to improve as much as I can so I can use this repo as a portfolio.
If you notice that my code is not clear, lacks documentation, is hard to read, or anything I can get better at, please feel free to tell me.

thank you

link to the repo:

Hi,

It seems nice. If anything it’s over-documented, which makes it harder to look at the code relevant code. For study purposes it’s good though. If you wanted to write a brief explanation of what it does in the beginning I would maybe make it more conceptual. Just some thoughts, good work!

4 Likes

Hello, @lealvcon, and welcome to the Codecademy Forums!

It all looks good. Note that the official documentation for object. __repr__ ( self ) states:

If at all possible, this should look like a valid Python expression that could be used to recreate an object with the same value (given an appropriate environment).

If you wish to follow this recommendation regarding the Node definition, you could try this:

    def __repr__(self):
        return "Node({})".format(repr(self.data))

Then, try this:

node1 = Node("spam")
node2 = Node(42)
node3 = Node(["apple", "berry", "cherry"])
print(node1)
print(node2)
print(node3)

Output:

Node('spam')
Node(42)
Node(['apple', 'berry', 'cherry'])

The output looks like valid Python expressions that could be used to recreate the Node objects.

However, there is still a wrinkle. How should we represent a Node that links to another ‘Node’? One possibility would be to rewrite two of the methods, as follows:

    def __init__(self, data, next_node=None):
        """assigns the data to the node while initializing its link as None"""
        self.data=data
        self.next_node=next_node

    def __repr__(self):
        return "Node({}, {})".format(repr(self.data), repr(self.next_node))

Then, you could do this:

node1.set_next_node(node2)
print(node1)

Output:

Node('spam', Node(42, None))

Is this a valid Python expression that could be used to recreate node1 with the link included? Let’s try this:

node1copy = Node('spam', Node(42, None))
print(node1)
print(node1copy)

Output:

Node('spam', Node(42, None))
Node('spam', Node(42, None))

They look the same.

You may not wish to follow this route, since it may introduce some additional complications, but it is something to think about.

4 Likes

thank you very much. I was looking for this type of recommendations/observations.
I’ll implement it right away.

1 Like

The big one would be the way you are doing your properties and having a get_property and set_property method in the class. The proper way to do this is to use the @property decorator like so:

class Node:
    def __init__(self, data, next_node=None):
        self.data = data
       
    @property    
    def data(self):
        return self._data
    
    @data.setter    
    def data(self, value):
        self._data = value

# In use
new_node = Node(1)
print(new_node.data)
# 1
new_node.data = 2
print(new_node.data)
# 2

You can see everything is now accessed via the one public interface of .data and we don’t have to explicitly use two different methods for the getter and setter. It also removes the need to add comments because the decorators are built-in and do it for you (unless you decide to add more logic), so cleans up the code in that sense too. One thing to note is that in the __init__ you do not put an _ before the property name but in the getter (@property) and setter (@propertyname.setter) you do.

Your __iter__ and __next__ can be simplified too, using a generator, like so:

def __iter__(self):
        node = self.head_node
        while node is not None:
            yield node
            node = node.next_node

# no need to explicitly do the __next__ method

Size in the linked list should again be a property without a setter.

I would add the option to choose where you insert the new node too, some examples would be at the start, end, before, after or between two nodes.

I haven’t looked at all of the code yet but a thing to note is that you probably want to add tests if you want to use it in a portfolio. I can see one case on your linked list that will cause unexpected behaviour for example:

linked_list = LinkedList()
linked_list.add_node(Node(1, Node(2, Node(3))))
for i in linked_list:
    print(i)
    # Node(1, Node(2, Node(3, None)))
    print(i.get_data())
    # 1
print(linked_list.get_size())
# 1

The above example will only ever print 1 in the for loop, despite the head pointing to another node. The reasons for this isn’t that hard to see, but clearly the behaviour of adding a node to the list that already had pre-existing links isn’t handled but is easy for a user to do.

If you are going to use the git repository as the portfolio, I would get used to branching and not working on master. Make a branch, do your work there and then merge it into master, that is what most people would expect to see you doing in the real world, so might as well show them you can do that. Pull requests aren’t really going to be expected if you are doing it on your own, but you could do them anyway and double check your work in one as a sanity check.

2 Likes

You may wish to perform some testing of linked Nodes if you do that, in order to take the following into account:

In particular, watch for behaviors resulting from the copying of Nodes, such as in the following, taken from a previous post:

node1copy = Node('spam', Node(42, None))

Also consider trying out @jagking’s recommendations. Again, work with the results by creating some linked Node objects for testing.