2
\$\begingroup\$

A while back I wrote a socket programmed chat in Python. This question isn't really about the program specifically, moreso on conventions of Python and my style of coding. I've only been using Python for a couple of months and any pointers would be appreciated.

Here's some code I'm curious about:

# in case of inappropriate arguments, remind user how to execute
if len(sys.argv) != 3:
    print("\nExecute the script as below:")
    print(" $ ./sock_client.py <IP Address> <Port>")
    print("For example: `./sock_server.py 127.0.0.1 8008`.")
    print("Additionally, ensure the socket server is live beforehand.\n")
    sys.exit(1)

In all of my Python projects so far, which all are ran from a terminal, I have a block of code like this at the top to ensure number of arguments, and if not, remind the user on how to execute. I don't know if this is good convention or not.

def send_user(m, u):
    """
    Sends a message to a single client.

    Parameter:
    m -- the message to send
    u -- the client to send to
    """
    for c in CLIENT_LIST:
        # only specific client/user
        if CLIENT_LIST[c] == u:
            c.send(bytes(m, "utf8"))

Here's an example docstring. Were I to have a return statement, it would look more like this, though:

def function(param1, param2):
    """
    What function does in a couple words.
    More detail if needed.

    Parameter:
    param1 -- some parameter
    param2 -- another parameter

    Return:
    -- some value

What is considered ideal convention for docstrings?

Here's the full repo: link.

\$\endgroup\$
3
\$\begingroup\$

Instead of always manually parsing sys.argv, you can use the standard library tool argparse. It automatically enforces the right usage and prints a nicely formatted usage if not. For your given example it would look like this:

import argparse

def ip_address(ip):
    # TODO: implement checking if IP is valid
    return ip

def parse_args(args=None):
    parser = argparse.ArgumentParser(description="Tool for server client connection.",
                                     epilog="Ensure the socket server is live before using this tool.")
    parser.add_argument("ip", type=ip_address, help="The IP address of the server")
    parser.add_argument("port", type=int, help="The port where the server listens.")
    return parser.parse_args(args)

I put it into a function so you can test it without having to run the whole code, by passing a list of strings to the function. It also allows setting the type of arguments, which can be any callable (so in the above example you need to have e.g a function called ip_address that can check if the argument is a valid IP address). The help messages generated by this example are like this:

$ python3 /tmp/sock_client.py 127.0.0.1
usage: sock_client.py [-h] ip port
sock_client.py: error: the following arguments are required: port

$ python3 /tmp/sock_client.py -h       
usage: sock_client.py [-h] ip port

Tool for server client connection.

positional arguments:
  ip          The IP address of the server
  port        The port where the server listens.

optional arguments:
  -h, --help  show this help message and exit

Ensure the socket server is live before using this tool.

You can also add optional arguments, arguments with multiple parameters, mutually exclusive arguments/argument groups. Check out the documentation for more examples.


Instead of iterating only over the keys of a dictionary, iterate over the keys and values at the same time if you need both of them. Also, don't use too short names. No matter how nice your docstring is, it is always better if your function signature already tells you everything you need to know.

def send_user(message, user):
    """
    Sends a message to a single client.

    Parameter:
    message -- the message to send
    user    -- the client to send to
    """
    for client, user_name in CLIENT_LIST.items():
        # only specific client/user
        if user_name == user:
            client.send(bytes(message, "utf8"))

Also, don't lie in your docstrings. This function sends a message to every client with the right name (i.e. value in the dictionary)! You can just break after you have found the user to ensure there is only one, or adapt the docstring to the reality of the code. In either case, add tests to ensure that your code does what it should.

Calling what appears to be a dictionary CLIENT_LIST is one of the reasons why you should not include the type in the variable. Here it is very misleading and probably wrong! Just call them CLIENTS or ALL_CLIENTS or even USER_NAMES.


There are two widely used docstring conventions (that I know of, i.e. in scientific computing). The first is the Google style and the other one the numpy style. Your code seems to be closer to the latter, but as long as you are consistent and include all necessary information you should be fine.

| 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.