A vector-based drawing package simulated on a text console

I had an exercise as below for an interview. I used two design patterns: factory and decorator. I put my code at the end of it.

How could I improve the solution?

Basic OOD & Solution Design Requirements

Develop a simple Visual Studio CONSOLE Application which simulates a vector-based drawing package.

Your application should support the following 5 drawing primitives (we'll call them widgets):

1) rectangle
2) square
3) ellipse
4) circle
5) textbox

The application should allow a user to Add a new widget to the drawing, stating the location and size/shape of the widget.

The location can be a standard x/y coordinate on an imaginary page. The size/shape depends on the widget, as follows:

• rectangle – width and height

• square – width

• ellipse –horizontal and vertical diameter

• circle – diameter

• textbox – bounding rectangle (i.e., the rectangle which surrounds the textbox; the text will be centred within this rectangle).

NOTE:

• there is no need to worry about rotation.
• integer units are fine for all dimensions.
• for the textbox you only need to configure the text to display (which will be rendered horizontally within the bounding rectangle). Don’t worry about font face /size / alignment, etc.
• as per our coding standards, please use a separate .cs file for each class.

Your console application should be able to 'print out' the current drawing by printing the key details of each widget (type, location, size/shape) to the console.

You don't need to actually render the widgets in any manner - we're just simulating a drawing package at this stage.

Your application can use the Console Application Main() method to test the drawing simulator by adding a hard-coded set of widgets to the drawing.

PLEASE ADD ONE OF EACH WIDGET TO YOUR DRAWING

** Important Note:
You DO NOT need to concern yourself with handling any input from the user. Our widgets will be hard-coded within the Main() method.

Time allowed: 2 hours.

My solution follows:

```
publ

Off the bat I see a couple of things with regards to MS published coding standards, although I'm not sure what their standards are.

I'm not sure why the X/Y locations are private. Seems to me that you'd want those properties exposed as public members with regular fields backing them and default values set. Exa:

private int _x = 0;
public int X { 
  get { return _x; }
  set { _x = value; }
}

As it stands now, the only way to get the location is through the GetLocation method which returns a string already formatted for display... not exactly what you want as that couples display to implementation which is a no-no.

Next public properties should always follow proper capitalization. You have:

public int size { get; set; }

which should be:

public int Size { get; set; }

Although, again, I'd set a default value or ensure some type of constraints on it.

Next, instead of having a generic try .. catch. I'd simply test if the object implemented the IDraw interface and, if so, call it's method. The try .. catch, especially with a generic catch, is lazy and for those whom performance is critical not a good thing.

Finally, I'm not sure that I would implement the print formatting within the classes themselves anyway. Again, that's coupling presentation with logic. Perhaps it would have been better to have another class whose job it is to emit the objects.

Finally/Finally, I would think 2 hours is probably 4 times longer than necessary to whip this out, even under pressure. Then again, I type fast.

All in all, if I was judging your entry against the requirements I'd estimate that you were a junior developer that needs some training. Yes, you met the requirements however the code itself would need additional work before going into a production type environment. If I was looking for a senior guy I wouldn't bother bringing you in for more questions.

Hope that helps. This answer is not meant to be bashing in any way but rather just a real world breakdown of what one person that routinely hires developers (not for them) sees wrong. So I hope it's taken in that vain.

Just my 0.02. Apart from the above points, especially since you presented it as a simple OOD exercise, I would have implemented a "Drawing" class with Print and Add methods instead of using a simple List.

Furthermore, your use of the new modifier in the Print method of Rectangle really annoys me, and would have made me look for someone else.

I think that I would have defined Shape as implementing IDraw, too, with a virtual version of Print printing just the position of the Shape (or even an abstract version of the method, if you prefer). Then you can override the method as you want in all your subclasses.

In the majority of cases having to use the new modifiers makes me look for bad design decisions.

StackExchange Code Review Q#17561, answer score: 4