Skip to main content
deleted 1 character in body
Source Link
Carcigenicate
  • 16.3k
  • 3
  • 35
  • 80

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.*

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.

And instead of having an is_empty method, it's more idiomatic to just have a __bool__ method so your stack can be treated as a boolean value. See this answer to see how that can be done.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.*

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.

And instead of having an is_empty method, it's more idiomatic to just have a __bool__ method so your stack can be treated as a boolean value. See this answer to see how that can be done.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.

And instead of having an is_empty method, it's more idiomatic to just have a __bool__ method so your stack can be treated as a boolean value. See this answer to see how that can be done.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()
added 258 characters in body
Source Link
Carcigenicate
  • 16.3k
  • 3
  • 35
  • 80

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.*

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.

And instead of having an is_empty method, it's more idiomatic to just have a __bool__ method so your stack can be treated as a boolean value. See this answer to see how that can be done.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.*

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.

And instead of having an is_empty method, it's more idiomatic to just have a __bool__ method so your stack can be treated as a boolean value. See this answer to see how that can be done.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()
added 8 characters in body
Source Link
Carcigenicate
  • 16.3k
  • 3
  • 35
  • 80

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic"". They""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start with two underscores are ""magic"". They have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()

The code is clean. My major concern with how it looks is that your naming doesn't follow PEP8. Names should be snake_case, not camelCase. So isEmpty should be is_empty.

With how it works, I'd work on giving it consistent behavior with other collections.

Right now, you have a sizeStack method for returning the size. This should really be __len__ instead:

def __len__(self):
    return len(self.stack)

Why? Because now you can do this:

stack = Stack()
stack.push(1)
stack.push(2)
stack.push(3)
print(len(stack))  # Prints 3

You can now check the length using len like you can with list and other built-ins. __len__ and other methods that start and end with two underscores are ""magic""; they have special meanings behind the scenes. Here for example, len just delegates to a classes's __len__ method. I'd recommend looking over that page I linked to.

isEmptys could also be made a little more idiomatic by making use of the fact that empty lists are falsey:

def is_empty(self):
    return not self.stack

The major advantage here is nearly all collections are falsey when empty. With how you have it now, if you change what underlying structure your class uses, you'll need to remember to update the is_empty method, or self.stack == [] will always fail.


And list actually already has a pop method that does what you want. Your pop can just be:

def pop(self):
    return self.stack.pop()
edited body
Source Link
Carcigenicate
  • 16.3k
  • 3
  • 35
  • 80
Loading
Source Link
Carcigenicate
  • 16.3k
  • 3
  • 35
  • 80
Loading