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

Chat using commands

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

Problem

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()

Solution

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)

Code Snippets

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

Context

StackExchange Code Review Q#57769, answer score: 3

Revisions (0)

No revisions yet.