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

Programming Challenge: Drawing Tool

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
programmingdrawingtoolchallenge

Problem

Introduction to the problem


You are free to implement any mechanism for feeding input into your solution. You should provide sufficient evidence with unit tests that your solution is complete. As a minimum, please use the provided test data to indicate that the solution works correctly. Any programming language can be used to solve the problem.


Drawing tool


You're given the task of writing a simple console version of a drawing program. At this time, the functionality of the program is quite limited but this might change in the future. In a nutshell, the program should work as follows:



-
Create a new canvas

-
Start drawing on the canvas by issuing various commands

-
Quit



At the moment, the program should support the following commands:


C w h - Should create a new canvas of width w and height h.


L x1 y1 x2 y2 - Should create a new line from (x1, y1) to (x2, y2). Currently only horizontal or vertical lines are supported. Horizontal and vertical lines will be drawn using the x character.


R x1 y1 x2 y2 - Should create a new rectangle, whose upper left corner is (x1, y1) and lower right corner is (x2, y2). Horizontal and vertical lines will be drawn using the x character.


B x y c - Should fill the entire area connected to (x, y) with "colour" c. The behaviour of this is the same as that of the "bucket fill" tool in paint programs.


Q - Should quit the program.

You can view test data and output here.

I would just like to know what, if anything, is wrong with the PHP in this answer that I wrote and submitted, things that would indicate that I'm not qualified for a senior developer position.

draw.class.php

```
pixels[$x][$args[1]] = $char;
} elseif($args[0] == $args[2]) { // vertical line
$line = range($args[1], $args[3]);
foreach($line as $y)
$this->pixels[$args[0]][$y] = $char;
} else {

Solution

-
I don't like the naming of consoleListener(). These should contain verbs; maybe listenOnConsole()

-
controller() is not only named poorly, it also does too much. This should be split to parseInput() and renderCanvas() or something.

-
$pixels isn't meaningful; it would be better if it was named $canvas. Your current $canvas could be $canvas_width and $canvas_height.

  • In fact, it would make sense to encapsulate that into a Canvas object.



-
$line = range(); foreach ($line) looks very Pythonic. I would personally prefer a simple for loop: for ($x = $x1; $x

-
Your signature for
drawLine() is confusing. Why even have a $args parameter? It would make much more sense as drawLines($x1, $y1, $x2, $y2, $colour).

-
Draw` is a poor class name.

-
The problem statement suggested that you add unit tests, but you didn't. To the evaluator, it would seem like you did the minimum amount of work.

Context

StackExchange Code Review Q#58767, answer score: 6

Revisions (0)

No revisions yet.