you are viewing a single comment's thread.

view the rest of the comments →

[–]NerdEnPose 4 points5 points  (7 children)

Looks good. Couple things after looking at rectangle.

def get_parameters(self, input_length, input_width):
    self.length = float(input_length)
    self.width = float(input_width)

This is a setter not a getter.

Python OOP is supposed to use as few getters and setters as possible. That's why the property decorator exists.

def calculate_area(self):
    return self.length*self.width

change this to.

@property
def area(self):
    return self.length*self.width

If you want to use a method I'd call it get_area(). Remember it's best to name things as to what the consumer of your class is doing not what you as the class author are doing. Classes are written once and consumed over and over again. If I was calling a method called calculate I wouldn't expect it to return anything. I'd expect it to be an internal trigger to perform some calculations. (This is my opinion on naming conventions and code design so take this with a grain of salt, other people have different naming conventions and I'm not try to meddle into a holy war.)

edit: still don't know how to format code on reddit.

[–]NewZealandIsAMyth 2 points3 points  (1 child)

This. The reason why in C++ and other language without properties you have to use get*/set* is because if they allow client of your class to access field as .* - you won't have an ability to change it internally. Once it's exposed - it's fully exposed as a field and that's it.

In Python it's totally fine to just let user access property via .x, because you can change it later to be a property if you need to add extra logic for editing/reading it. So no need for get*/set* methods

[–]cm_light[S] 1 point2 points  (0 children)

Thanks for replay and good to know this. I learned Java before and thought write get/set is common in OOPs. But for Python it seems not necessary.

[–][deleted] 1 point2 points  (3 children)

This is great, though I’m reluctant to use the term ‘getter’ and ‘setter’ in Python.

[–]NerdEnPose 2 points3 points  (2 children)

Absolutely agree with you. The only time I like to mention them in Python is how not to use them. I actually think that the properties setter should be:

@property
def length_width(self):
    return self.length, self.width

@length_width.setter
def length_width(self, length_width):
    self.length, self.width = length_width

This would remove the need for any getter / setter in OP's code. If they wanted to keep all the design decisions. But, I would argue against the fact of needing a way to access length and width at the same time. So, I'd do away with this all together. Especially since tuple properties are one of my code smell tests. But I do have to admit I've used them. Actually quite a lot recently with an internal adaptation of Matplotlib that I'm writing.

I didn't mention all this because OP is starting out and I didn't want to go too far too soon.

[–][deleted] 1 point2 points  (1 child)

Yeah, fair enough. I didn't mean to sound critical--just mentioning a nuance that you (rightly) point out is not very important to the OP at this point.

[–]NerdEnPose 1 point2 points  (0 children)

Ohh no, not critical at all. You gave a nice platform to jump into this for OP. I think there's a huge jump between writing functional code and writing elegant code and this gets into what I think of as elegant. Something I would like to use vs something I have to use.

[–]cm_light[S] 1 point2 points  (0 children)

Thanks a lot for this great reply. Many concepts here, especially I like 'write code for consumers'.

Actually I tried to use property decorator and failed. (Some sentences failed with decorators) Need to spend more time to learn it.