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

Tool to execute commands to draw lines and rectangles

Submitted by: @import:stackexchange-codereview··
0
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)

Solution

First off, there should be two blank lines between top-level functions/classes/code blocks. You've also styled the class 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 line

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