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

Primitive Python P2P Socket Chat

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

Problem

Here is my code for a Python script for a peer to peer chat system on a network using sockets. I know there are some things that could be changed so there would be less repetition in the code and that some variables could be global, but what I am asking is if there are any issues with the code's functionality; it seems to work properly on my home network but will only connect (not receive messages) on my school network. Because it establishes a connection there, it looks like the port is not being blocked by the internal firewall.

```
#!usr/bin/env python

import socket
import threading
import select
import time
import datetime

def main():

class Chat_Server(threading.Thread):
def __init__(self):
threading.Thread.__init__(self)
self.running = 1
self.conn = None
self.addr = None
def run(self):
HOST = ''
PORT = 8080
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind((HOST,PORT))
s.listen(1)
self.conn, self.addr = s.accept()
# Select loop for listen
while self.running == True:
inputready,outputready,exceptready \
= select.select ([self.conn],[self.conn],[])
for input_item in inputready:
# Handle sockets
message = self.conn.recv(1024)
if message:
print "Daniel: " + message + ' (' + datetime.datetime.now().strftime('%H:%M:%S') + ')'
else:
break
time.sleep(0)
def kill(self):
self.running = 0

class Chat_Client(threading.Thread):
def __init__(self):
threading.Thread.__init__(self)

Solution

Regular review.

-
main is bloated. All the class definitions do net belong there.

-
Creating a Chat_Client instance in a server branch (and vice versa) looks strange. My guess it is a result of a Text_Input design, which assumes that both server and client exist. This is of course wrong. A Text_Input instance should have just one "sink" where the text input would go:

class Text_Input(threading.Thread):
        def __init__(self, sink):
            threading.Thread.__init__(self)
            self.sink = sink
            self.running = 1
        def run(self):
            while self.running == True:
              text = raw_input('')
              self.sink.send(text)


and server initialization becomes

chat_server = Chat_Server()
text_input = TextInput(chat_server)


Same goes for a client. Both server and client should implement send method.

-
Network initialization should be moved into constructors. After that, run and send method (same for client and server) should be factored out into a separate Chatter class which client and server inherit from.

-
What is the purpose of time.sleep(0)? If the system misbehaves without it, then there is a bug to be fixed, not masked. Hint: you should select for outputready only when there is a message to be sent.

Code Snippets

class Text_Input(threading.Thread):
        def __init__(self, sink):
            threading.Thread.__init__(self)
            self.sink = sink
            self.running = 1
        def run(self):
            while self.running == True:
              text = raw_input('')
              self.sink.send(text)
chat_server = Chat_Server()
text_input = TextInput(chat_server)

Context

StackExchange Code Review Q#64628, answer score: 6

Revisions (0)

No revisions yet.