0

I am trying to understand class structure.

The example is a pizza order with __size and __toppings_list. I can pass size easy enough but it wont let me passing toppings as a list I keep getting

size = small
toppings = small

No matter how I try to evoke the __string__ print function it just won't play nice.

class Pizza:

    def __init__(self, size = "small", topping_list = []):
        self.__size = size
        self.__topping_list = topping_list

    def __string__(self):
        string = "Size is" + self.__size + ' and Topping is ' + self.__topping_list
        return string

    def set_size(self, size):
        self.__size = size

    def get_size(self):
        return self.__size

    def set_topping(self, topping):
        self.__topping_list

    def get_topping(self):
        return self.__size

    def add_topping(self, topping):
        self.__topping_list

    def get_cost(self):
        cost = 0
        size = pizza.get_size()
        toppings = pizza.get_topping()
        totalToppings = len(toppings)

        if size == "small":
            cost += 10
        elif size == "medium":
            cost += 12
        elif size == "large":
            cost +=14        
        cost += totalToppings * 2

        return cost


pizza = Pizza("small",["meat"])
size = pizza.get_size()
toppings = pizza.get_topping()
print(size)
print(toppings)
pizza_string = str(pizza)
print(pizza_string)
Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
  • 5
    `def get_topping(self): return self.__size` <- see a problem in that code? – Aran-Fey Oct 19 '18 at 12:23
  • 3
    See also: https://stackoverflow.com/questions/1132941/least-astonishment-and-the-mutable-default-argument. And `def set_topping(self, topping): self.__topping_list` won't do much either. – user2390182 Oct 19 '18 at 12:24
  • 1
    There are a lot of issues here to address: `print(value)` will use `str(value)`, which in turn looks for `__str__`. `__string__` plays no role in that path. Python has **no privacy model**, names starting with double underscores do not hide internal state, that only renames attributes to help avoid clashes with subclasses. Don't use `__` to start names unless you are writing a framework API to be extended by third-party developers. – Martijn Pieters Oct 19 '18 at 12:34
  • 2
    Python doesn't need to use getters and setters, as it is trivial to update a class later on to use a `@property` if you find you need one at that time, unlike Java, where you can't later on make that switch so it makes sense to always use setters and getters there. Don't try to use Python as if it is Java. If you had not used getters and setters you'd have been far less likely to run into your simple error here, where your `get_topping` getter returns `__size`. – Martijn Pieters Oct 19 '18 at 12:35
  • I found my problem .. ty for the responses :-) – Scot Henderson Oct 19 '18 at 12:36
  • 1
    You also have a problem with `topping_list = []` in your `__init__` signature, that list is *shared between all instances*, see ["Least Astonishment" and the Mutable Default Argument](//stackoverflow.com/q/1132941). – Martijn Pieters Oct 19 '18 at 12:37
  • 1
    Compare your code with https://gist.github.com/mjpieters/0c6b1982fdebe90086a655b88277b00b, and think about what the differences are. You don't need all that extra code, really. – Martijn Pieters Oct 19 '18 at 12:41
  • In short I had to write a class using 1. __size The size of the pizza (small, medium or large) and 2. __topping_list A list of toppings (strings). The class Pizza should also contain the following methods: __init__() , __str__() , get_size(), set_size(), get_topping_list(), set_topping_list(), add_topping(), get_cost() <-- this one has small pizza is $10, medium $12, large $14 && $2 per topping .. can I ask how a pro would have written this. I am struggling on how to print the __string__ big time as you can see :-) – Scot Henderson Oct 19 '18 at 12:42
  • @ScotHenderson: it's sad to see that courses are teaching to code with Python as if it's Java then. Do you feel you can share what course this is? – Martijn Pieters Oct 19 '18 at 12:47
  • @Martin Pieters .. that was an excellent read and like the code .. I see now that making things private (i thought you had to so no malicious user could set the variable instance outside the class) is not a requirement for Python. Thank you for teaching me something i will use from now on :-) – Scot Henderson Oct 19 '18 at 12:52
  • The course is Bachelor of Information Technology and Data Analytics at UniSA https://online.unisa.edu.au/degrees/bachelor-of-information-technology-and-data-analytics#degreeoverview – Scot Henderson Oct 19 '18 at 12:54

1 Answers1

4

There's a lot of small weird things going on with your code.

First of all, fix get_topping method (as pointed in comments), re-create your instance and it will work — this is for fixing your main issue.

Minor things, but highly recommended:

  • using [empty] list as default argument is not a good idea, until you know what you're doing. List is mutable and arguments is "bound" to function object. So changing that default value once will affect all subsequent calls.

  • thing you're trying to achieve with __string__ looks like mistake while implementing __str__

  • please, think twice (or more) when you're using __attr. Probably it looks like a good idea, if you're experienced with languages like Java, but in python its generally a bad idea to "hide" attributes this way. This still will be available for users, while making your class hard to inherit — and non-collaborative classes isn't fun to work with. If you want to identify non-public interface — consider using single underscore (_size). If you not sure if this should be public — make it public. You can always make it @property without breaking interface. Same applies to get_... methods — there's no point to have code like this, if you're retrieving attribute without having attributes. Just having self.size attribute and using it is fine. If you're 100% sure you don't want to make this editable after instantiation, make it

    def __init__(...):
        self._size = ...
    @property
    def size(self):
        return self._size
    
Slam
  • 8,112
  • 1
  • 36
  • 44
  • TY :-) .. I am going to go read up on @property .. and have a quite chat with my uni lecturer ;-) about the professional feedback here .. I learnt more in 5 minutes with you people than in the last week covering python classes – Scot Henderson Oct 19 '18 at 13:07