2

Could someone look over this and let me know if i've butchered anything. It's just a simple class definition in python, but I don't have any local devs to make sure I'm using the language right. Using 'self' so much feels awkward and I don't know if it is needed.

import time
import pexpect
from datetime import datetime
import deviceModelconfigs
from kafka import KafkaClient, KeyedProducer, HashedPartitioner, RoundRobinPartitioner

class Device():
    def __init__(self, roomName, ip, model):
        self.model = model
        self.ip = ip
        self.roomName = roomName
        self.codes = deviceModelconfigs.setCodes(self.model)
        self.powerStatus = 'na'
        self.signalStatus = 'na'
        self.networkStatus = 'na'

    def connect(self):
        self.child = pexpect.spawn('telnet '+ self.ip + ' 2001')


    def sendMessage(self):
        print ("devices", self.roomName, str("{\"timestamp\":\"" +  str(datetime.utcnow().isoformat()) +
                "\",\"roomName\":\""+ self.roomName +
                    "\",\"powerStatus\":\""+ self.powerStatus +
                    "\",\"signalStatus\"\""+ self.signalStatus +
                    "\",\"networkStatus\":\""+ self.networkStatus +
                "\",\"deviceModel\":\""+ self.model +
                "\",\"deviceType\":\""+ self.codes['deviceType'] + "\"}"))

    def testPower(self):
        try:
            self.child.sendline(self.codes['pwrStCmd'])
            self.child.expect(self.codes['pwrStOnRe'], timeout=5)
            status = 'on'
        except:
            try:
                self.child.sendline(self.codes['pwrStCmd'])
                self.child.expect(self.codes['pwrStOffRe'], timeout=5)
                status = 'off'
            except:
                status = 'na'
        return status

    def testAll(self):
        self.powerStatus = Device.testPower(self)
        Device.sendMessage(self)
        self.child.terminate(True)

jeff = Device('FM1-200','10.1.174.12','WD510U')
jeff.connect()
jeff.testAll()
schumacherj
  • 123
  • 4
  • 2
    For future reference, [CodeReview.SE](http://codereview.stackexchange.com/) is the place to post code that works but you feel could use a few dozen extra sets of eyes. –  Feb 18 '15 at 00:06
  • Go to the bottom of the page and view the whole list. There are tons. –  Feb 18 '15 at 00:11

1 Answers1

4

You need self when you want to refer to the instance of a class within the class itself.

For example, an instance method can call other instance methods or properties or use instance fields.

If you have used other object oriented languages, you may have a habit of using this. Python's self is similar (while not identical) to this in other languages.

The usage of self in your sample code looks correct.


Note that:

  • You may consider replacing strings like "{\"timestamp\":\"" by the more readable '{"timestamp":"'.

  • Consider using string formatting. Two most popular ways to format strings in Python is % and .format. If you are unsure which one should you use, this answer on Stack Overflow may help.

  • You use strings to describe statuses. While this is an acceptable approach, you may also be interested in Python's enums.

  • You import four types from kafka. Have you considered using just import kafka? This answer on Programmers.SE explains the difference between import and from ... import.

  • In testPower, you have code duplication. Consider extracting lines containing sendline and expect statements in a separate method.

Arseni Mourzenko
  • 134,780
  • 31
  • 343
  • 513