Chat using commands

Question

I have been learning Python and was trying to make a chat system which can be run by commands. This is my first attempt to write the code. Does this make sense or is it wrong usage of classes?

There is a User class which will contain the user of the chat. The Message class is used to send messages and count their length.

class User:
 def __init__(self):
 self.users = []
 def add_remove_user(self,input):
 command, name = input[:1], input[1:]
 if command == "+":
 if not name in self.users:
 self.users.append(name)
 elif command == "-":
 if name in self.users:
 self.users.remove(name)
class Message:
 def __init__(self,user):
 self._messages=[]
 self._user=user
 def __parseMessage__(self,message):
 parsedMessage=tuple(message.split(":"))
 return parsedMessage
 def send_message(self,inputMessage):
 user,message = self.__parseMessage__(inputMessage)
 if user in self._user.users:
 self._messages.append(message)
 def sent_messages_count(self):
 count=0
 for message in self._messages:
 count += len(message)
 return count
class MessageClient:
 def __init__(self):
 self.user=User()
 self.message=Message(self.user)
 def send(self,inputMessage):
 if inputMessage[0] == "+" or inputMessage[0] == "-":
 self.user.add_remove_user(inputMessage)
 else:
 self.message.send_message(inputMessage)
 def sent_message_length(self):
 return self.message.sent_messages_count()

• jonrsharpe: Which version of Python, and is there some test to run it with?

Answer 1

A few comments regarding style (mainly based on PEP8):

• Use for spaces for each level of indentation
• Use a space around binary operators like =
• Use a space after a , for an argument
• Do not use the camelCase naming style: parsedMessage -> parsed_message, inputMessage -> input_message.
• Try not to use built-in function names (input)
• For private methods names use a single underscore (there's a use case for double leading underscore in complex inheritance, but that's not the case here)
• Use x not in y instead of not x in y
• Write docstrings (PEP257)

Some other comments:

• Why have an add_remove_user method? Having both add_user and remove_user methods would be more readable.
• For the users you probably want to use a set
• Be careful with length message.split(':'). You probably want message.split(':', 1) to avoid problems with messages that contain colons in them.
• sent_message_length and sent_message_count don't seem to implement what I would expect from their name

• Use and instead of nesting if statements. For example, instead of this:

  if command == "+":
   if not name in self.users:
   self.users.append(name)
  

  write this:

  if command == "+" and name not in self.users:
   self.users.append(name)
  

• Paritosh: that is some good point , how about class structure. should user class do the parsing of input message or it should be part of some other class like inputvalidator
• jcollado: I think there's still some work to be done in the code. In my opinion the classic design is having a client class, a server class, a channel class and a message class. The client objects connect to the server, join one or more channels and send messages through a channel. The server object forwards the messages to all clients connected to a channel. The channel objects might be just a list of users or maybe act as a message queue of pending messages. The message objects might be just containers or do some encoding/decoding.