patternpythonMinor
Tool to execute commands to draw lines and rectangles
Viewed 0 times
rectanglescommandsdrawtoollinesandexecute
Problem
I submitted a project and never heard back and I am wondering if my coding style is okay. At the company I work I get feedback, but I'd like to hear from the outside world.
It's a drawing tool - the input is in a text file (each on its own line):
C 20 4
L 1 2 6 2
L 6 3 6 4
R 16 1 20 3
It's called as so: python drawingtool.py < input.txt
C = Canvas create, L = Line, R = Rectangle
```
from collections import defaultdict
import logging
import sys
NEWLINE = '\n'
DATA_SEP = '/'
PARAM_SEP = ' '
DATA_POS = 1
PARAM_POS = 0
class Canvas(object):
_created = 0
def __init__(self, cols, rows):
self.rows = rows
self.cols = cols
Canvas._created += 1
self.surface = self._create_blank_canvas()
def _create_blank_canvas(self):
row_frame_char = '-'
col_frame_char = '|'
canvas = []
top_bottom = [row_frame_char for col in xrange(self.cols+2)]
middle = [self._middle_char(col, col_frame_char, self.cols+1) for col in xrange(self.cols+2)]
for row in xrange(self.rows+2):
if row == 0 or row == self.rows+2:
canvas.append(list(top_bottom))
else:
canvas.append(list(middle))
return canvas
def _middle_char(self, col, col_frame_char, max):
return col_frame_char if col == 0 or col == max else ' '
def print_canvas(self):
for row in self.surface:
print ' '.join(row)
def valid_pos(self, x, y):
ret_val = False
if not self._boundary(x, y) and self._on_canvas(x, y):
ret_val = True
return ret_val
def _boundary(self, x, y):
return (x == 0 or x == self.rows+1) or (y == 0 or y == self.cols+1)
def _on_canvas(self, x, y):
return (x > 0 and x 0 or y 'C':
attempts += 1
logging.error('Cant draw no canvas created skipping') #args[0].split(' ')
else:
request = command_q.pop(0)
It's a drawing tool - the input is in a text file (each on its own line):
C 20 4
L 1 2 6 2
L 6 3 6 4
R 16 1 20 3
It's called as so: python drawingtool.py < input.txt
C = Canvas create, L = Line, R = Rectangle
```
from collections import defaultdict
import logging
import sys
NEWLINE = '\n'
DATA_SEP = '/'
PARAM_SEP = ' '
DATA_POS = 1
PARAM_POS = 0
class Canvas(object):
_created = 0
def __init__(self, cols, rows):
self.rows = rows
self.cols = cols
Canvas._created += 1
self.surface = self._create_blank_canvas()
def _create_blank_canvas(self):
row_frame_char = '-'
col_frame_char = '|'
canvas = []
top_bottom = [row_frame_char for col in xrange(self.cols+2)]
middle = [self._middle_char(col, col_frame_char, self.cols+1) for col in xrange(self.cols+2)]
for row in xrange(self.rows+2):
if row == 0 or row == self.rows+2:
canvas.append(list(top_bottom))
else:
canvas.append(list(middle))
return canvas
def _middle_char(self, col, col_frame_char, max):
return col_frame_char if col == 0 or col == max else ' '
def print_canvas(self):
for row in self.surface:
print ' '.join(row)
def valid_pos(self, x, y):
ret_val = False
if not self._boundary(x, y) and self._on_canvas(x, y):
ret_val = True
return ret_val
def _boundary(self, x, y):
return (x == 0 or x == self.rows+1) or (y == 0 or y == self.cols+1)
def _on_canvas(self, x, y):
return (x > 0 and x 0 or y 'C':
attempts += 1
logging.error('Cant draw no canvas created skipping') #args[0].split(' ')
else:
request = command_q.pop(0)
Solution
First off, there should be two blank lines between top-level functions/classes/code blocks. You've also styled the class
Secondly, the class
While I understand your logic behind naming your actual drawing functions
You also shouldn't be using
I'm also noticing a pattern with your inputted commands. The always form a structure somewhat like this:
Finally, I'd recommend adding docstrings to your code. Right now, nothing in your code file has docstrings. Here's an example of docstrings in use:
Drawing_data. It should be appropriately renamed to DrawingData.Secondly, the class
Drawing_data is completely pointless. Is there any reason why self.data = self.read_data_lines() can't be part of the DataUtils class?While I understand your logic behind naming your actual drawing functions
C, L, R, and B, it'd be better if you gave them better names, like canvas, and lineYou also shouldn't be using
% for string formatting. If you're running Python 2.6 or higher, string formatting with % is deprecated, and str.format should be used instead. Here's an example of how to use str.format:# str.format without positional or named arguments
print "{} {}".format("Hello", "world")
# str.format with positional arguments
print "{1} {0}".format("world", "Hello")
# str.format with named arguments
print "{word1} {word2}".format(word1="Hello", word2="world")I'm also noticing a pattern with your inputted commands. The always form a structure somewhat like this:
[name] [integer] .... While it works with a small example like this, if you have bigger commands, I'd recommend giving them actual command names, and doing something like this: canvas width:20 height:20. It makes things a lot clearer to read.Finally, I'd recommend adding docstrings to your code. Right now, nothing in your code file has docstrings. Here's an example of docstrings in use:
"""
This docstring is at the top of the file,
it should describe the purpose of this file.
"""
class MyClass(object):
"""
Describe your class and it's arguments here.
"""
...
def my_func( ... ):
"""
Describe your function and it's arguments here.
"""
...Code Snippets
# str.format without positional or named arguments
print "{} {}".format("Hello", "world")
# str.format with positional arguments
print "{1} {0}".format("world", "Hello")
# str.format with named arguments
print "{word1} {word2}".format(word1="Hello", word2="world")"""
This docstring is at the top of the file,
it should describe the purpose of this file.
"""
class MyClass(object):
"""
Describe your class and it's arguments here.
"""
...
def my_func( ... ):
"""
Describe your function and it's arguments here.
"""
...Context
StackExchange Code Review Q#96578, answer score: 2
Revisions (0)
No revisions yet.