HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple Python chat

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplechatpython

Problem

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 =

Solution

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:

  • 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.



  • Method names should also be in snake_case.



  • 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.

-
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" + "-" * ...


-
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


-
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, ...):
    ...

Code Snippets

LoadConnectionInfo('Connected with: ' + str(addr) + '\n---------------------------------------------------------------')
"\n" + "-" * ...
while 1:
    pass
while True:
    pass
range(0,len(EndFiltered), 1)

Context

StackExchange Code Review Q#111496, answer score: 3

Revisions (0)

No revisions yet.