I'm currently writing a python script which needs some simple networking. After some playing around with sockets, this emerged, but as this is my first foray into sockets I'd really like a second opinion.
Edit: I know my script is not PEP8 compliant, as this is just an experiment. I posted it here to get Feedback about my usage of Sockets, and if there are any pitfalls or corner cases I overlooked.
Package Format:
datalength+1 0円 data 0円
import socket
sep: bytes = b'0円'
minpacketsize: int = 10
def sendpacket(sock: socket.socket, data: bytes) -> None:
send: bytes = str(len(data) + 1).encode('utf-8') + sep + data + sep
if len(send) < minpacketsize:
raise ValueError("Packet too small!")
sock.sendall(send)
def recivepacket(sock: socket.socket) -> bytes:
datalen = 0
header: bytearray = bytearray()
while not datalen:
header += sock.recv(1)
if header.endswith(sep):
try:
datalen = int(header[:-1].decode('utf-8'))
except UnicodeDecodeError or ValueError as e:
raise ValueError("Error while decoding header!") from e
if len(header) == 10:
raise ValueError("Could not find Header!")
rawdata: bytes = sock.recv(datalen)
if rawdata.endswith(sep):
return rawdata[:-1]
else:
raise ValueError("Could not find Packet end!")
1 Answer 1
Docstrings
You should include a docstring at the beginning of every method, class, and module you write. This will allow documentation to identify what your code is supposed to do. It also helps external text editors, such as VSCode, to display what types of parameters your functions accept, and a short description of your method.
Error or Error
This line
except UnicodeDecodeError or ValueError as e:
should be this
except (UnicodeDecodeError, ValueError) as e:
Your current line of code raises this warning from pylint
:
Exception to catch is the result of a binary "or" operation
If you need to handle multiple errors, you should group them into a tuple.
Method Names
According to PEP-8 Function Name Conventions, your method names sendpacket
and recivepacket
should really be send_packet
and receive_packet
.
Unnecessary else
This section of code
if rawdata.endswith(sep):
return rawdata[:-1]
else:
raise ValueError("Could not find Packet end!")
should be this
if rawdata.endswith(sep):
return rawdata[:-1]
raise ValueError("Could not find Packet end!")
Since you are returning a value if the condition is satisfied, the ValueError line will not run. And if the condition is not satisfied, it will simply move to the next line. Therefore, the else
is unnecessary and should be removed.
Updated Code
"""
Description of this module goes here
"""
import socket
sep: bytes = b'0円'
minpacketsize: int = 10
def send_packet(sock: socket.socket, data: bytes) -> None:
"""
Description of this method goes here
:param sock -> socket.socket: Socket to send the data over\n
:param data -> bytes: Data to send over the socket
:return: None
"""
send: bytes = str(len(data) + 1).encode('utf-8') + sep + data + sep
if len(send) < minpacketsize:
raise ValueError("Packet too small!")
sock.sendall(send)
def receive_packet(sock: socket.socket) -> bytes:
"""
Description of this method goes here
:param sock -> socket.socket: Socket to receive the data over
:return: bytes
"""
datalen = 0
header: bytearray = bytearray()
while not datalen:
header += sock.recv(1)
if header.endswith(sep):
try:
datalen = int(header[:-1].decode('utf-8'))
except (UnicodeDecodeError, ValueError) as e:
raise ValueError("Error while decoding header!") from e
if len(header) == 10:
raise ValueError("Could not find Header!")
rawdata: bytes = sock.recv(datalen)
if rawdata.endswith(sep):
return rawdata[:-1]
raise ValueError("Could not find Packet end!")
-
\$\begingroup\$ Thanks for catching the exception handling error. I don't want to sound rude, but I know PEP8. This is just a quick and dirty experimentation script, and I posted it here to get feedback on my usage of sockets, not my PEP8 compliance. I will edit my Question to clarify. \$\endgroup\$Xtrem532– Xtrem5322019年11月15日 19:46:33 +00:00Commented Nov 15, 2019 at 19:46
Explore related questions
See similar questions with these tags.