patternpythonMinor
TCP client and server supporting six simple commands
Viewed 0 times
sixsimpleservercommandstcpclientsupportingand
Problem
For a class, I was given an assignment to code a simple TCP connection between a server and a client. Once the TCP handshake is done, the client sends inquiries to the server. It's a 2 second conversation, if that.
I need some feedback on my code. Please keep in mind that I am new to Python. The prof kinda threw the class to the wolves on this.
This is the server code:
```
import socket
import time
import sys
#from thread import *
def Main():
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
host = socket.gethostname()
port = 8008
s.bind((host, port))
print ("Port, that's a wine right? " + str(port))
s.listen(1)
c, addr = s.accept()
print ("200 SRP version 1.0 ready")
print ("Connection from: " + str(addr))
while True:
data = c.recv(1024)
data = str(data).upper()
print ("sending: " + str(data))
c.send(data)
print ("from connected user: " + str(data))
print ("Welcome, would you like a jelly baby?")
if not data:
break
if True:
if data.startswith('HELO'):
try:
c.sendall("210 HELO " + host + " This is Dalek High Command. You will obey all orders without question!")
except:
c.sendall("510 You are unauthorized! You will be exterminated!")
break
elif data.startswith('REQTIME'):
try:
c.sendall ("220 I obey! The time is " + time.strftime("%H%:M%:S"))
except:
c.sendall ("520 The time is unavailable! Exterminate!")
break
elif data.startswith('REQDATE'):
try:
c.sendall ("230 I obey! The date is " + time.strftime("%Y/%m/%d"))
I need some feedback on my code. Please keep in mind that I am new to Python. The prof kinda threw the class to the wolves on this.
This is the server code:
```
import socket
import time
import sys
#from thread import *
def Main():
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
host = socket.gethostname()
port = 8008
s.bind((host, port))
print ("Port, that's a wine right? " + str(port))
s.listen(1)
c, addr = s.accept()
print ("200 SRP version 1.0 ready")
print ("Connection from: " + str(addr))
while True:
data = c.recv(1024)
data = str(data).upper()
print ("sending: " + str(data))
c.send(data)
print ("from connected user: " + str(data))
print ("Welcome, would you like a jelly baby?")
if not data:
break
if True:
if data.startswith('HELO'):
try:
c.sendall("210 HELO " + host + " This is Dalek High Command. You will obey all orders without question!")
except:
c.sendall("510 You are unauthorized! You will be exterminated!")
break
elif data.startswith('REQTIME'):
try:
c.sendall ("220 I obey! The time is " + time.strftime("%H%:M%:S"))
except:
c.sendall ("520 The time is unavailable! Exterminate!")
break
elif data.startswith('REQDATE'):
try:
c.sendall ("230 I obey! The date is " + time.strftime("%Y/%m/%d"))
Solution
I would recommend that you read PEP 8, the Python style guide.
PEP 8 says:
Use 4 spaces per indentation level.
You use four spaces in some places, but in others you use eight. At the very least you should be consistent, but it is best if you consistently use four spaces. In fact, looking at your code, your indentation is completely shot. I spy with my little eye an
One line up you defined
You convert
You have some duplicate code there. That is, it would be duplicate if you were consistent with what you wanted. There are two things that change in these: The string given to the first
You exit the program and then expect to be able to close
I'm guessing that the
PEP 8 says:
Use 4 spaces per indentation level.
You use four spaces in some places, but in others you use eight. At the very least you should be consistent, but it is best if you consistently use four spaces. In fact, looking at your code, your indentation is completely shot. I spy with my little eye an
IndentationError in the elif data.startswith('REQDATE'): block. Your break statements in those if blocks are not consistent either.print("sending: " + str(data))One line up you defined
data as a string. Python objects don't just drift into new types. They don't change unless you change them. You don't need the str part here. Just do this:print("sending: " + data)You convert
data to a string in several other places, but there also it is useless. That's like translating a German sentence into English and then translating that sentence into English every time I want to use it. What is the purpose of translating English into English?if True:True is always True, so that if block will always be executed. Why do you use an if block if you always want to execute what is inside?if data.startswith('HELO'):
try:
c.sendall(...)
except:
c.sendall(...)
break
elif data.startswith('REQTIME'):
try:
c.sendall(...)
except:
c.sendall(...)
break
...You have some duplicate code there. That is, it would be duplicate if you were consistent with what you wanted. There are two things that change in these: The string given to the first
c.sendall() and the string given to the second c.sendall(). I would create a dictionary:messages = {
'HELO': (
"210 HELO " + host + " This is Dalek High Command. You will obey all orders without question!",
"510 You are unauthorized! You will be exterminated!"),
'REQTIME': (
"220 I obey! The time is " + time.strftime("%H%:M%:S"),
"520 The time is unavailable! Exterminate!"),
'REQDATE': (
"230 I obey! The date is " + time.strftime("%Y/%m/%d"),
"530 The date is unavailable! Exterminate!"),
'ECHO': (
"240 Data received! " + data.upper(),
"540 ECHO! I cannot obey! Assist! Assist! Cannot comply! Exterminate!"),
'REQIP': (
"250 IP has been identified! " + host,
"550 Alert! I cannot obey! Your IP cannot be retrieved!"),
}
for key, value in messages.items():
if data.startswith(key):
try:
c.sendall(value[0])
except:
c.sendall(value[1])
break
else:
if data.startswith('BYE'):
try:
c.sendall ("600 Alonzy! Bowties are cool! See you later!")
except:
breaksys.exit()
c.close()You exit the program and then expect to be able to close
c? Perhaps that's a debugging call that you forgot to take out, but sys.exit() is unnecessary here.for x in range(0, 5):
if x == 0:
data = 'helo'
s.send(data)
elif x == 1:
data = 'reqtime'
elif x == 2:
data = 'reqdate'
elif x == 3:
data = 'echo'
elif x == 4:
data = 'reqid'
elif x == 5:
data = 'bye'
s.send(data)
print(str(data))
x = x + 1I'm guessing that the
s.send(data) and below lines were meant to be inside of the for loop. If they were, there is a much simpler way to do this:for data in ("helo", "reqtime", "reqdate", "echo", "reqid", "bye"):
s.send(data)
print(data)Code Snippets
print("sending: " + str(data))print("sending: " + data)if data.startswith('HELO'):
try:
c.sendall(...)
except:
c.sendall(...)
break
elif data.startswith('REQTIME'):
try:
c.sendall(...)
except:
c.sendall(...)
break
...messages = {
'HELO': (
"210 HELO " + host + " This is Dalek High Command. You will obey all orders without question!",
"510 You are unauthorized! You will be exterminated!"),
'REQTIME': (
"220 I obey! The time is " + time.strftime("%H%:M%:S"),
"520 The time is unavailable! Exterminate!"),
'REQDATE': (
"230 I obey! The date is " + time.strftime("%Y/%m/%d"),
"530 The date is unavailable! Exterminate!"),
'ECHO': (
"240 Data received! " + data.upper(),
"540 ECHO! I cannot obey! Assist! Assist! Cannot comply! Exterminate!"),
'REQIP': (
"250 IP has been identified! " + host,
"550 Alert! I cannot obey! Your IP cannot be retrieved!"),
}
for key, value in messages.items():
if data.startswith(key):
try:
c.sendall(value[0])
except:
c.sendall(value[1])
break
else:
if data.startswith('BYE'):
try:
c.sendall ("600 Alonzy! Bowties are cool! See you later!")
except:
breaksys.exit()
c.close()Context
StackExchange Code Review Q#124227, answer score: 2
Revisions (0)
No revisions yet.