improve this newbie code/nested functions in Python?

Paul Hankin paul.hankin at gmail.com
Sat Mar 21 09:07:41 CET 2009


On Mar 20, 3:21 am, Esmail <ebo... at gmail.com> wrote:
> Hi,
>
> I'm new to writing Python code. This is a simple client I wrote, it
> works, but I feel it doesn't look as clean as it could. Can anyone
> make suggestions how to streamline this code?
>
> Also, I am using two nested functions, it seems that nested functions
> aren't used that much in Python - is that correct? And if so, how
> come?
>
> thanks,
>
> Esmail
>
> ps: I realize there is minimal error checking/exception handling.
>
> #!/usr/bin/env python
>
> import sys
> from socket import *
> from threading import Thread
>
> class Client(object):
>     def __init__(self, host="localhost", port=5555, name = "esmail"):
>         self._host = host
>         self._port = port
>         self._name = name
>         self._address=(self._host, self._port)
>         self._sock=socket(AF_INET, SOCK_STREAM)
>         self._sock.connect(self._address)
>         self._parent = self
>         self._keepGoing = True
>
>     def info(self):
>         return self._host, self._port, self._name
>
>     class Listener(Thread):
>        def __init__(self, parent, tname="listener"):
>            Thread.__init__(self,name = tname)
>            self._parent = parent
>            print self._parent._host
>
>        def run(self):
>
>            while self._parent._keepGoing:
>                m = self._parent._sock.recvfrom(1024)
>                print m[0]
>
>     class Speaker(Thread):
>        def __init__(self, parent, tname = "speaker"):
>            Thread.__init__(self,name = tname)
>            self._parent = parent
>            self._parent._sock.send(self._parent._name + "\n")
>
>        def run(self):
>
>            while(True):
>                m = raw_input("-> ")
>                if m == "bye":
>                    self._parent._sock.send(self._parent._name + " is
> signing off.\n")
>                    self._parent._sock.send("bye\n")
>                    self._parent._keepGoing = False
>                    break;
>                else:
>                    self._parent._sock.send(m + "\n")
>
> def main():
>
>     if len(sys.argv) == 4:  # prog name + 3 args
>         host = sys.argv[1]
>         port = int(sys.argv[2])
>         name = sys.argv[3]
>         c = Client(host, port, name)
>     else:
>         c = Client()
>
>     print "Client connecting to - host=%s  port=%d   name=%s" % c.info
> ()
>
>     s = Client.Speaker(c)
>     s.start()
>
>     l = Client.Listener(c)
>     l.start()
>
> main()

I would decouple the speaker and the listener from the client, and
make the client interface more abstract. Simple and descriptive
interfaces   can make code dramatically easier to understand.

class ClientListener(Thread):
  def __init__(self, client, ...):
    ...

  def run(self):
    while True:
       m = self.client.receive_message()
       print m

class ClientSpeaker(Thread):
  def __init__(self, client):
    client.connect()
  ...
  def run(self):
    while True:
      m = raw_input()
      if m == 'bye':
        self.client.disconnect()
        ...
      else:
        self.client.send_message(m)

The 'connect' method can print the client name, the 'disconnect' can
say 'bye' and set _KeepGoing. receive_message can extract the
interesting bit from the socket read. Hope you get the idea.

This way, you don't use the inner workings of client in the Listener
and Speaker classes, and don't violate the 'law of demeter'. If
nothing else, it makes the speaker and listener much easier to read.
If you later want to test these classes, you'll find it a ton easier,
since you can mock the client class.. but perhaps that's a little too
advanced.

HTH
--
Paul Hankin



More information about the Python-list mailing list