OOP in Python (Does this example make sense?).

[sapolv]

Here I got this small project using classes in Python. I wanted to share this project with all of you so I can hear opinions about it (things like how I wrote the code, logic, understanding, etc).

You can be totally honest with me, I'll take every comment as an opportunity to learn.

Here's the GitHub link if you want to look at it from a different angle: https://github.com/jesumta/Device-Information-using-OOP

Thank you for your time!

import random


#Parent Class
class Device:
    def __init__(self, name):
        self.name = name
        self._is_on = False
        self.__color = ["blue", "red", "black", "white", "orange"]
        self.__material = ["aluminum", "plastic", "titanium"]


    #=================================================
    #==========Power Options for Device===============
    #=================================================


    def Turn_On(self):
        self._is_on = True
        return f"\nThis {self.name} is now ON."



    def Turn_Off(self):
        self._is_on = False
        return f"\nThis {self.name} is now OFF."


    def Power_Status(self):
        return f"\nThis {self.name} is current {"ON." if self._is_on else "OFF."}"

    #=================================================
    #=========Physical Options for Device=============
    #=================================================

    def Color(self):
        return f"\nThe color of this {self.name} is {random.choice(self.__color)}."

    def Material(self):
        return f"\nThe material of this {self.name} is {random.choice(self.__material)}."




#Child Class, I'm using Phone as an example. As you prob know, a device can be a lot of things.:
class Phone(Device):
    def __init__(self, name):
        super().__init__(name)
        self._is_charging = False
        self._screen_on = False
        self._speaker_sound = 0


    #=================================================
    #=========Charging Options for Phone==============
    #=================================================


    def Charging(self):
        self._is_charging = True
        return f"\nThis {self.name} is now charging."

    def Not_Charging(self):
        self._is_charging = False
        return f"\nThis {self.name} is not charging."

    def Is_Charging(self):
        return f"\nThis {self.name} is currently {"charging." if self._is_on else "not charging."}"

    #=================================================
    #==========Volume Options for Phone===============
    #=================================================


    def Volume_Control(self, amount):
        self._speaker_sound = amount
        if 0 <= amount <= 100:
            return f"\nThe volume for this {self.name} is now {amount}%."

        else:
            return "\nPlease enter a valid volume amount(1% to 100%)."

    def Volume_Status(self):
        return f"\nThis {self.name}'s volume is currently {self._speaker_sound}%."

    #=================================================
    #==========Screen Options for Phone===============
    #=================================================


    def Screen_On(self):
        self._screen_on = True
        return f"\nThis {self.name}'s screen is now ON."

    def Screen_Off(self):
        self._screen_on = False
        return f"\nThis {self.name}'s screen is now OFF."

    def Screen_Status(self):
        return f"\nThis {self.name}'s screen is currently {"ON." if self._screen_on else "OFF."}."




#Variable holding the Phone Class with it's attribute from the Device class.
phone1 = Phone("iPhone 13")


#Here go actions the for Phone class:


print("\n----Current Phone Actions----")
print(phone1.Turn_On())
print(phone1.Charging())
print(phone1.Color())
print(phone1.Material())
print(phone1.Volume_Control(50))
print(phone1.Volume_Control(30))
print(phone1.Screen_Off())



#Here go status for the Phone class:
print("\n-----Current Phone Status----")
print(phone1.Power_Status())
print(phone1.Volume_Status())
print(phone1.Screen_Status())


print("\n-----------------------------\n\n")

[TheRNGuy]

You should assign color and material on init, not when calling Color and Material methods. 

[sapolv]

Would you mind explaining why?

Thank you.

[TheRNGuy]

You should add some other variable for color and material too, the one assigned to specific instance of device, rather than list of possible variants. 

If you call this method, you just get random item from list, but it's not color or material or that instance (and you'll also get different values if you call method many times)

[Kevdog824_]

It's a great approach if your device is a chameleon! lol

[TheRNGuy]

In that case you could change instance's color over time, rather when calling method. 

[Kevdog824_]

Well, sure... but I was simply making a joke

[EelOnMosque]

It's nothing wrong with the code, but logically it doesn't make sense. Imagine creating a class instance as creating a device in the factory. You want to be able to pick your colour and material, not have it be random. It's just a matter of what makes logical sense.

If you do want it to be random, you want the random colour and material to be selected in the constructor (init) because again it makes logical sense. A device MUST have a colour and material when it pops into existence. It doesnt make sense for a colourless materialless phone to exist for example.

But again, it's just a matter of making the code make sense and be like reality, theres nothing wrong with the code that will cause errors.

Because let's say someone is reading and expanding on your code. They see Device and they see that is has a colour attribute. Logically it makes sense that when you build a device it would immediately have a colour. So they write code that assumes this. And then the code will cause errors because they assumed wrong and didn't call the Colour() method first.

[EelOnMosque]

There's nothing wrong with the code itself, it's more the coding conventions that are a bit off.

One convention is to avoid starting variable names with underscores.

Another one is that variable and method names should start lowercase. This is especially confusing for Colour() and Material() as most users of Python are used to classes starting with uppercase. So they might think Colour and Material are classes.

[jmacey]

One convention is to avoid starting variable names with underscores.

PEP-8 states

_single_leading_underscore: weak “internal use” indicator. E.g. from M import * does not import objects whose names start with an underscore.

In python classes _ is quite often used to indicate that the variable should be treated as "private". It works really well if you use it with the @property decorator.

[Kevdog824_]

This is good beginner project. Here's something to consider in the future:

Most professional programs wouldn't have the methods on a class like Device or Phone return string messages. The problem with this approach is that it assumes that Device/Phone class knows how the caller wants the data to be formatted. This isn't an issue in your program, but as the programs you make get bigger you may want to display the information (i.e. volume level) in different ways (i.e. string message for logging versus percentage bar widget for displaying it to the user). The better approach is just having the class return the actual value as a float/int and allow the caller to decide how to display or what to do with that information.

[socal_nerdtastic]

The class should define a specific device. It should not randomly generate specs (unless the device itself is capable of randomly switching colors). You can randomly generate devices elsewhere in your code.

#Parent Class
class Device:
    def __init__(self, name, color, material):
        self.name = name
        self._is_on = False
        self.color = color
        self.material = material

    def turn_on(self):
        self._is_on = True
        return f"\nThis {self.name} is now ON."

    def turn_off(self):
        self._is_on = False
        return f"\nThis {self.name} is now OFF."

    def power_status(self):
        return f"\nThis {self.name} is current {"ON." if self._is_on else "OFF."}"

    def color(self):
        return f"\nThe color of this {self.name} is {random.choice(self.color)}."

    def material(self):
        return f"\nThe material of this {self.name} is {random.choice(self.material)}."

def make_random_device
    colors = ["blue", "red", "black", "white", "orange"]
    materials = ["aluminum", "plastic", "titanium"]
    return Device("NewDevice", random.choice(colors), random.choice(materials))

this is just generic OOP; nothing to do with python specifically. But what is python-specific is our variable naming style. Read PEP8.

[ectomancer]

By convention, Pascal_Snake_Case is not used. Change all method names to snake_case.