3
\$\begingroup\$

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()
\$\endgroup\$
  • \$\begingroup\$ how about socketserver? \$\endgroup\$ – Aries_is_there Mar 10 '19 at 15:57
  • \$\begingroup\$ 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\$ – markspace Jan 23 at 16:24
  • \$\begingroup\$ 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\$ – markspace Jan 23 at 16:26
1
\$\begingroup\$

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 becomes server_running
  • Use a geometry manager like grid for better display
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.