People say that the code in my implementation is generally obscure. Can you point me what looks bad?

import time
import string
from queue import Queue
from channel import Channel
from client import Client

__author__ = 'mark'
import socket
import connection
from threading import Thread

SPECIAL = "[]\`_^{|}"
ALLOWED_NICKNAME = string.digits + string.ascii_letters + SPECIAL + "-"
# noinspection PyUnboundLocalVariable
ALLOWED_CHANNEL = "".join([chr(x) for x in range(128)]).replace("\0", "").replace("\7", "").replace("\13", "").replace("\10", "")\
    .replace(" ", "").replace(",", "").replace(":", "")

class IRCServer(object):
    _message_queue = Queue()
    _sock = None
    _connections = None
    _bind_address = None
    running = None
    _clients = None
    _channels = None
    _nick_change_failed = []

        This prevents memory leak when the client who had error was disconnected
    def _dead_check_thread(self):
        while self.running:
            for conn in self._nick_change_failed:
                for client_conn in self._clients:
                    if client_conn == conn:

    def _ping_check_thread(self):
        while self.running:
            for conn in self._clients.keys():
                if self._clients[conn].last_pinged >= 250:
                    print("[DBG] %s disconnected because ping has timed-out" %
                    self.disconnect(conn, "Ping timeout: 250 seconds")
                    self._clients[conn].last_pinged += 1

    def _ping_thread(self):
        while self.running:
            for client_conn in self._clients.keys():
                except IOError:
                    self.disconnect(client_conn, "Remote host closed the connection")

    def _message_thread(self):
        while self.running:
            for conn in self._connections:
                    for msg in conn.get_messages():
                except IOError:  # We couldn't read from socket, thus the connection is dead.
                    print("[DBG] can't read from connection %s" % str(conn))

    def _message_handler_thread(self):
        self._nick_change_failed = []
        while self.running:
            msg = self._message_queue.get(True)
            text = msg.get_data()
            conn = msg.get_connection()
            args = text.replace("\r", "").replace("\n", "").split(" ")
            command = args[0].upper()
            command_args = args[1:]
            if command == "NICK":
                if len(command_args) = 2:
                    self._clients[conn].send("PONG %s" % command_args[1])
            elif command == "QUIT":
                for client in self._clients.values():
                    for channel in self._clients[conn].channels:
                        if channel in client.channels:
                            client.connection.send("%s QUIT %s" % (self._clients[conn].get_identifier(),
                                                                   " ".join(command_args)))
                del self._clients[conn]  # It's dead
            elif command == "TOPIC":
                if len(command_args)  users and 
               services on  servers"
       252    RPL_LUSEROP
              " :operator(s) online"
       253    RPL_LUSERUNKNOWN
              " :unknown connection(s)"
              " :channels formed"
       255    RPL_LUSERME
              ":I have  clients and 
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 251 %s :There are %d users and 0 services on 1 servers" % (self.name, nick, len(self._clients)))
        self._clients[conn].send(":%s 252 %s 0 :operator(s) online" % (self.name, nick))
        self._clients[conn].send(":%s 253 %s 0 :unknown connection(s)" % (self.name, nick))
        self._clients[conn].send(":%s 254 %s %d :channels formed" % (self.name, nick, len(self._channels)))
        self._clients[conn].send(":%s 255 %s :I have %d clients and 1 servers" % (self.name, nick, len(self._clients)))

    def _send_no_channel(self, conn, chan_name):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 403 %s %s :No such channel" % (self.name, nick, chan_name))

    def _send_no_user(self, conn, target):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 401 %s %s :No such nick/channel" % (self.name, nick, target))

    def _send_not_enough_parameters(self, conn, command):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 461 %s %s :Not enough parameters" % (self.name, nick, command))

    def _send_unknown_command(self, conn, command):
        nick = self._clients[conn].get_nick()
        self._clients[conn].send(":%s 421 %s %s :Unknown command" % (self.name, nick, command))

    def _send_nickname_in_use(self, conn, nick):
        conn.send(":%s 433 %s :Nickname already in use" % (self.name, nick))

    def _send_erroneous_nickname(self, conn, nick):
        conn.send(":%s 432 %s :Erroneous nickname" % (self.name, nick))

    def disconnect(self, conn, message):
        client = self._clients[conn]
        identifier = client.identifier if client.identifier else client.nick
        self._send_to_related(conn, ":%s QUIT :%s" % (identifier, message))
            self._clients[conn].send("ERROR :Closing link [%s]: Disconnected" % conn.address)
        except IOError:
        del self._clients[conn]

    def _send_to_related(self, conn, msg, ignore_self=False):
        clnt = self._clients[conn]
        for client in self._clients.values():
            if ignore_self and client != conn:

            for channel in client.channels:
                if channel in clnt.channels:

    def _channel_broadcast(self, conn, chan, msg):
        for client in self._clients.values():
            if client.nick == self._clients[conn].nick:
            if chan in client.channels:

    def _nick_in_use(self, nick):
        :param nick: Nickname of client
        :type nick: str
        :return: True if a client with that name exists, else False
        for client in self._clients.values():
            if client.nick == nick:
                return True
            return False

    def _valid_nick(self, nick):
        if not all(c in ALLOWED_NICKNAME for c in nick) or len(nick) > 9 or\
                not len(nick):
            return False
        return True

    def _set_nick(self, conn, nick, ident=None):
        if self._nick_in_use(nick):
            self._send_nickname_in_use(conn, nick)
            return False
        elif not self._valid_nick(nick):
            self._send_erroneous_nickname(conn, nick)
            return False
            if not conn in self._clients:
                self._clients[conn] = Client(connection=conn, nick=nick)
                old_nick = self._clients[conn].nick
                self._clients[conn].nick = nick
                self._send_to_related(conn, "%s NICK %s" % (old_nick, nick))
                if ident:
                    self._clients[conn].ident = self._clients[conn].get_nick() + "!" + \
                                                                   ident + "@" + self.name

            return True

    def __del__(self):
        del self._sock

In the method IRCServer._ping_check_thread, you seem to be checking if a client's connection with the server has timed out.

However, you seem to be re-inventing the wheel there. The python socket library already has two functions that handle with timeouts called socket.setdefaulttimeout and socket.settimeout.

Both of these functions take a float parameter that specifies how long before a the socket.timeout error is raised and the user is disconnected. However, the method socket.setdefaulttimeout sets the timeout for all sockets.

I recommend that you use this rather than re-invent the wheel.

Note: I believe by using one of the settimeouts, the client will automatically disconnect. However, I don't know for sure.

I may be missing something, but it doesn't look like anywhere in your code are you using the socket library.

In that case, I don't see why you imported it.

I will look more into this post later; right now, I do not have the time.


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.