Designing classes in python

Question

I am designing a class which produces information like cpu usage,disk usage,Mem usage,...... for 3 systems,lets say data centers which has many workstations and work stations has many PCs.So the CPU usage and the other parameters are required for all the 3 levels(Datacenters, Workstations, Pcs). Please suggest is the below class design correct or how it is to be designed

EDIT

class Datacenters:
 def __init__(self,name,location,cpu,mem):
  self.name=name
  self.location=location
  self.cpu=cpu
  self.mem=mem

 def  getparam(self):
  return self.name,self.location ,self.cpu,self.mem

 def  getname(self):
  return self.name

class WS(Datacenters):
 def __init__(self,name,location,cpu,mem,obj):
  #datacentername = Datacenters.__init__(self) #To which data center it is associated
  obj.getname() #To which data center it is associated

  self.name=name
  self.location=location
  self.cpu=cpu
  self.mem=mem

def  getparam(self,obj):
  return self.name,self.location ,self.cpu,self.mem,obj.getname()

def  getpcname(self):
  return self.name

class Pcs(WS):
 def __init__(self,name,location,cpu,mem,obj):
  obj.getpcname() #To which WS it is  associated
  self.name=name
  self.location=location
  self.cpu=cpu
  self.mem=mem

 def  getparam(self,obj):
  return self.name,self.location ,self.cpu,self.mem,obj.getpcname()


a = Datacenters("dc1","Bl1",20,30)
print a.getparam()
b = WS("WS1","Bl1",20,30,a)
print b.getparam(a)
c = Pcs("PC1","Bl1",20,30,b)
print c.getparam(b)

That's all bad code. Looks like you should read some basic introduction to Python before you try to use it. — Chris Morgan
– Chris Morgan, Commented Dec 19, 2010 at 8:40
Your example is meaningless. It's not valid code and doesn't even express what you're thinking, not even close. — Falmarri
– Falmarri, Commented Dec 19, 2010 at 8:43

Lennart Regebro · Accepted Answer · 2010-12-19 18:57:16Z

3

Much of your code is pointless. I removed the lines that isn't needed:

class Datacenter:
    def __init__(self,name,location,cpu,mem):
        self.name=name
        self.location=location
        self.cpu=cpu
        self.mem=mem

class Workstation:
    def __init__(self,name,location,cpu,mem,datacenter):
        self.name=name
        self.location=location
        self.cpu=cpu
        self.mem=mem
        self.datacenter=datacenter

class Computer:
    def __init__(self,name,location,cpu,mem,workstation):
        # This line does nothing:
        self.name=name
        self.location=location
        self.cpu=cpu
        self.mem=mem
        self.workstation=workstation

You don't need to write accessor methods in Python. Just access the attributes directly. Also, the line obj.getname() did nothing.

And of course, as pointed out by others here, since when does a Datacenter have a CPU? And doesn't a Computer have the same location as a Workstation? Then it doesn't need the location. Probably your code should look like this:

class Datacenter:
    def __init__(self,name,location):
        self.name=name
        self.location=location

class Workstation:
    def __init__(self,name,location,datacenter):
        self.name=name
        self.location=location
        self.datacenter=datacenter

class Computer:
    def __init__(self,name,cpu,mem,workstation):
        # This line does nothing:
        self.name=name
        self.cpu=cpu
        self.mem=mem
        self.workstation=workstation

   def location(self):
       return self.workstation.location

   def datacenter(self):
       return self.workstation.datacenter

edited Dec 19, 2010 at 18:57

answered Dec 19, 2010 at 18:01

Lennart Regebro

173k45 gold badges230 silver badges254 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Paweł Prażak Over a year ago

original class names doesn't make sense, they shouldn't be plural

Lennart Regebro Over a year ago

@Paweł Prażak: This is true as well, I'll update. WS is also a crappy name. :)

Rajeev Over a year ago

Thanks this was elpful +1 for the explaination

AndiDog · Accepted Answer · 2010-12-19 11:35:50Z

Some suggestions:

Use 4-spaces indentation
Use properties instead of getters, provide single properties, not a single one for all parameters (because parameters might be added later or by subclassing). In order to use properties, derive the base class from object (new style classes)
Use singular class names, "Datacenter" instead of "Datacenters", because a single instance doesn't represent multiple datacenters, right?
Don't call parameters "obj"
So, a datacenter has a CPU? And a workstation is a datacenter, and gets a datacenter as an argument? Whaaat?

These points were mostly about coding style. I can't really help you with your use case - hope you get the point that your class hierarchy is totally screwed?!

Falmarri · Accepted Answer · 2010-12-19 08:27:35Z

2

What does this even mean?

   class WS(Datacentres):
      name
      location
      cpu
      mem

Your design sounds wrong anyway. If a Datacenter has workstations, why would workstation inheit from datacenter? A workstation isn't a datacenter.

http://en.wikipedia.org/wiki/Has-a

answered Dec 19, 2010 at 8:27

Falmarri

48.7k43 gold badges158 silver badges196 bronze badges

2 Comments

Rajeev Over a year ago

But my point is that WS needs to have a reference to which Datacenter it belongs

user470379 Over a year ago

@Rajeev Then you need to either explicitly describe in English what you're thinking, or else provide valid Python code that shows what you're thinking. Based on what you've provided we can't tell what your design is. The line class WS(Datacentres): is Python code to start declaration of a new class called WS that inherits from Datacentres. If it's meant to be interpreted another way, we need to be told how.

khachik · Accepted Answer · 2010-12-19 08:36:27Z

1

You shouldn't use inheritance here in that way, since Pcs is not WS and WS is not Datacentres.

You can make a class UnitInfo which has all that fields (name, cpu...) and use the instances of that class in DataCentres, WS, Pcs, because there is "has" relationship here (and not "is").

Or, make a class called Unit (or Units, I don't understand why you use plural forms), and inherit from that class. Unit "has" "name, "cpu-usage" and so on. "Pc" is a Unit. Maybe Unit is not the appropriate name for that class though.

answered Dec 19, 2010 at 8:36

khachik

28.8k10 gold badges63 silver badges98 bronze badges

Collectives™ on Stack Overflow

Designing classes in python

4 Answers 4

3 Comments

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related