This is a real live chatroom that I have made with Tkinter
I posted a question before about making a chatroom but the problem that I had with that one was that only one user could connect to the server so I restarted and have made this one. I just want suggestions on how I can improve it.
All suggestions will be greatly appreciated
Thanks
Server.py
import socket, threading
host = socket.gethostname()
port = 4000
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.bind((host,port))
s.listen()
clients = {}
addresses = {}
print(host)
print("Server is ready...")
serverRunning = True
def handle_client(conn):
try:
data = conn.recv(1024).decode('utf8')
welcome = 'Welcome %s! If you ever want to quit, type {quit} to exit.' % data
conn.send(bytes(welcome, "utf8"))
msg = "%s has joined the chat" % data
broadcast(bytes(msg, "utf8"))
clients[conn] = data
while True:
found = False
response = 'Number of People Online\n'
msg1 = conn.recv(1024)
if msg1 != bytes("{quit}", "utf8"):
broadcast(msg1, data+": ")
else:
conn.send(bytes("{quit}", "utf8"))
conn.close()
del clients[conn]
broadcast(bytes("%s has left the chat." % data, "utf8"))
break
except:
print("%s has left the chat." % data)
def broadcast(msg, prefix=""):
for sock in clients:
sock.send(bytes(prefix, "utf8")+msg)
while True:
conn,addr = s.accept()
conn.send("Enter username: ".encode("utf8"))
print("%s:%s has connected." % addr)
addresses[conn] = addr
threading.Thread(target = handle_client, args = (conn,)).start()
Client.py
import socket,threading,tkinter
host = input("Enter server name: ")
port = 4000
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
address = (host,port)
def echo_data(sock):
while True:
try:
msg = sock.recv(1024).decode('utf8')
msg_list.insert(tkinter.END, msg)
except OSError:
break
def send(event=None):
msg = my_msg.get()
my_msg.set("")
s.send(bytes(msg, "utf8"))
if msg == "{quit}":
s.close()
top.quit()
def on_closing(event=None):
my_msg.set("{quit}")
send()
top = tkinter.Tk()
top.title("Chat Room")
messages_frame = tkinter.Frame(top)
my_msg = tkinter.StringVar()
my_msg.set("Type your messages here.")
scrollbar = tkinter.Scrollbar(messages_frame)
msg_list = tkinter.Listbox(messages_frame, height=15, width=100, yscrollcommand=scrollbar.set)
scrollbar.pack(side=tkinter.RIGHT, fill=tkinter.Y)
msg_list.pack(side=tkinter.LEFT, fill=tkinter.BOTH)
msg_list.pack()
messages_frame.pack()
entry_field = tkinter.Entry(top, textvariable=my_msg)
entry_field.bind("<Return>", send)
entry_field.pack()
send_button = tkinter.Button(top, text="Send", command=send)
send_button.pack()
top.protocol("WM_DELETE_WINDOW", on_closing)
address = (host,port)
s.connect(address)
threading.Thread(target=echo_data, args = (s,)).start()
tkinter.mainloop()
1 Answer 1
This is a nice snippet which makes it useful for teaching! Here are some points:
Make imports explicit
Though import socket, threading
is valid in Python, importing in two lines improves readability
import socket
import threading
Two lines after imports
Add two lines after imports. From this:
import socket
import threading
host = socket.gethostname()
...
to this:
import socket
import threading
host = socket.gethostname()
...
Constants in caps
port = 4000
should be PORT = 4000
Use string formatting
From this:
"%s has left the chat." % data
to this:
"{} has left the chat.".format(data)
In case of curly braces, you escape using {{}}
as in the following case:
'Welcome {}! If you ever want to quit, type {{quit}} to exit.'.format(data)
Broadcast before handle_client
Since in handle_client
you use broadcast
, define it first
def broadcast(msg, prefix=""):
...
def handle_client(conn):
...
Add a message function
enclose:
bytes(msg, "utf8")
in a function called message
:
def message(text):
return bytes(text, "utf8")
then it becomes neater to use:
broadcast(message('hi'))
More explicit messages:
- 1) Server message
When first connecting, the server console states for me:
jPC
Server is ready...
And then when running clients, you get asked:
Enter server name:
I had to deduce that jPC is my server name. Modifying to the following might be more explicit:
Server name: jPC
Server is ready...
- 2) Enter username
Enter username in the textbox
might be a better message. Coupled with the fact that you did not use a placeholder for the entry, users are confused.
- 3) Quiting without username
If someone quits without setting a username the server says:
{quit} has left the chat.
Adding a default id for clients might be better
{
'<id2>': {
'username': None,
'connection_ip': '192.168.56.1:50325'
},
'<id2>': ...
}
you can use the uuid module for id or use the ip itself as id
Add placeholder effect
Add a placeholder effect by adding the line:
entry_field.bind("<FocusIn>", lambda args: entry_field.delete('0', 'end'))
Setting the font color to gray completes the effect.
Miscellaneous
- Use snake case for variables.
serverRunning
becomesserver_running
- Use a geometry manager like grid for better display
data = conn.recv(1024)
I don't speak Python but if this limits the number of characters that can be received it's going to be a problem. Look into some sort of streaming (character oriented) interface for sockets, trying to deal with buffers and buffer sizes directly is just going to be a headache. \$\endgroup\$threading.Thread(target = handle_client, args...
Launching one thread per person connected is wasteful and will limit the scaleability of your server. Look into implementing your net task with one thread: docs.python.org/3/library/selectors.html \$\endgroup\$