Simple Python chat

Question

I am working on a simple console chat in Python. I started from this Winforms application, removed all the forms (so that the chat could run on Windows and Linux) and did some refactoring (the "click events" did not exist any more, it needed a workaround).

The new client code:

import thread
from MinimalNetworkTools import *
HOST = str(get_internal_ip())
PORT = 8011
s = socket(AF_INET, SOCK_STREAM)
buffered_messages = ''
def UploadData():
 global s
 print 'listening for your input'
 while 1: 
 buffered_messages = raw_input()
 FlushMessages(s,buffered_messages)
def ReceiveData():
 global s
 thread.start_new_thread(UploadData,())
 try:
 s.connect((HOST, PORT))
 LoadConnectionInfo('[ Succesfully connected ]\n---------------------------------------------------------------')
 except:
 LoadConnectionInfo('[ Unable to connect ]')
 return
 while 1:
 try:
 data = s.recv(1024)
 except:
 LoadConnectionInfo('\n [ Your partner has disconnected ] \n')
 break
 if data != '':
 LoadOtherEntry(data)
 else:
 LoadConnectionInfo('\n [ Your partner has disconnected ] \n')
 break
thread.start_new_thread(ReceiveData,())
while 1:
 pass

The new server code:

import thread
from MinimalNetworkTools import *
s = socket(AF_INET, SOCK_STREAM)
HOST = gethostname()
PORT = 8011
conn = ''
s.bind((HOST, PORT))
def UploadData():
 global conn
 print 'listening for your input'
 while 1: 
 buffered_messages = raw_input()
 FlushMessages(conn,buffered_messages)
def GetConnected():
 s.listen(1)
 global conn
 print 'Waiting for connection...'
 conn, addr = s.accept()
 LoadConnectionInfo('Connected with: ' + str(addr) + '\n---------------------------------------------------------------')
 thread.start_new_thread(UploadData,())
 while 1:
 try:
 data = conn.recv(1024)
 LoadOtherEntry(data)
 except:
 LoadConnectionInfo('\n [ Your partner has disconnected ]\n [ Waiting for him to connect..] \n ')
 GetConnected()
 conn.close()
thread.start_new_thread(GetConnected,())
while 1:
 pass

And here are the methods shared by both codes:

from Tkinter import *
from socket import *
import urllib
import re
def get_external_ip():
 site = urllib.urlopen("http://checkip.dyndns.org/").read()
 grab = re.findall('([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)', site)
 address = grab[0]
 return address
def get_internal_ip():
 return str(gethostbyname(getfqdn()))
def FilteredMessage(EntryText):
 EndFiltered = ''
 for i in range(len(EntryText)-1,-1,-1):
 if EntryText[i]!='\n':
 EndFiltered = EntryText[0:i+1]
 break
 for i in range(0,len(EndFiltered), 1):
 if EndFiltered[i] != "\n":
 return EndFiltered[i:]+'\n'
 return ''
def LoadConnectionInfo(EntryText):
 if EntryText != '':
 print 'Connection info : ' + EntryText
def LoadMyEntry(EntryText):
 if EntryText != '':
 print '\t\t -> Sent!'
def LoadOtherEntry(EntryText):
 if EntryText != '':
 print 'Other : ' + EntryText,
def FlushMessages(s,buffered_messages):
 if len(buffered_messages) > 0 : 
 EntryText = FilteredMessage(buffered_messages)
 LoadMyEntry(EntryText) 
 s.sendall(EntryText)

My questions are:

• How can I make it cleaner?
• how bad is the thread safety?

Answer 1

To start off, I'd highly recommend that you check out Python's official style guide, PEP8. From just taking a quick glance at your code, I can see quite a few glaring issues regarding style. Here's a list of the major ones:

1. Variable and argument names should be in snake_case, not PascalCase or camelCase. If the variable is constant, it should be in UPPER_SNAKE_CASE.
2. Method names should also be in snake_case.
3. You should have two blank lines between each method, rather than one or none.

While the following issues aren't necessarily style-related, they're still good things to consider for writing clean quality code.

1. Instead of writing out a repeating string of characters, like you've done here:

   LoadConnectionInfo('Connected with: ' + str(addr) + '\n---------------------------------------------------------------')
   

   You can use the multiplication operator, *, to multiply a single character, like this:

   "\n" + "-" * ...
   

2. It's generally considered to be better to use plain True/False values in loops rather than what you've done here:

   while 1:
    pass
   

   The reasoning being that someone reading your code will be able to understand it more clearly, and see what the value is being used for. The above loop would become this:

   while True:
    pass
   

3. You don't need to specify the start and the step with range if you're starting at zero, and have a step of one. This means that this:

   range(0,len(EndFiltered), 1)
   

   Could be written like this:

   range(len(EndFiltered))
   

---

Never ever use a try/except construct with an except block that's not catching any specific error, like you've done here:

try:
 data = conn.recv(1024)
 LoadOtherEntry(data)
except:
 LoadConnectionInfo('\n [ Your partner has disconnected ]\n [ Waiting for him to connect..] \n ')
 GetConnected()

There are a fair amount of things that could go wrong with this, so I'll just prattle off a few of the major ones:

• Bad errors, like the Python SystemError that indicates something has gone wrong internally, go unnoticed.
• An error occurs that you don't want to catch, and variables you may have intended to set to a different value underneath the try block don't get set, and you'll have a helluva time figuring out why.
• An exception like KeyboardInterrupt, when raised, should exit the program regardless, but with a bare except clause, it won't.

There are many more bad things that could happen, but you get the idea. Don't do it. ;)

If you do need to catch multiple exceptions rather than just one, you can write your except clause like this:

except (FooBarException, DuckException, ...):
 ...