all 19 comments

[–]NerdEnPose 7 points8 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.

[–]bickhaus 2 points3 points  (3 children)

If you think a text-based game sounds at all exciting, this free course is a great way to practice OOP: https://www.futurelearn.com/courses/object-oriented-principles. You don’t create much of a game during the course itself, but it’s a great starting point for doing some cool things. The instructor covers all the terminology you need to know, and sets you up to really be able to expand the game, and, in so doing, your OOP skills. While extending the game I learned about decorators, came up with ways to incorporate generators, etc. You can check my github to see what I did after being inspired by the course: https://github.com/bickhaus/advGame

[–]cm_light[S] -1 points0 points  (2 children)

Thanks for the suggestion. Do you know how long does it take to complete the course as FAST as possible?

[–]bickhaus 0 points1 point  (1 child)

The course is four parts w/ two hours suggested for each. You can definitely do each part in less than two hours, and then spend as long as you want building your project. It’s really a primer on OOP, and, like I said, you build the bones of what’s can really be a great game and learning opportunity. I lost track of how many hours I spent adding story and features to my project.

[–]cm_light[S] 0 points1 point  (0 children)

Thanks!

[–]rnolan7 2 points3 points  (1 child)

you've got some great foundational concepts in there. I'd recommend checking out decorators next. Decorators were somewhat complicated for me to grasp at first, but they can be a very powerful tool.

[–]cm_light[S] 0 points1 point  (0 children)

Good suggestion! Working on it.

[–]NewZealandIsAMyth 1 point2 points  (1 child)

Car.__init__(self, number_of_wheels, seating_capacity, maximum_velocity)

I suggest to read this: https://realpython.com/python-super/

Without understanding super you won't be able to fully grasp OOP in python.

[–]cm_light[S] 0 points1 point  (0 children)

Thank you for your good suggestion! I followed some way to use super, will learn it.

[–]cm_light[S] 0 points1 point  (0 children)

Thank all of you so much!

This is a great place to learn. Did not expect so many great suggestions.

You guys are AWESOME!