The software should be able to:

  1. Draw shapes on a drawing
  2. Save drawings
  3. Load drawings

Adding a new shape should have a limited effect on existing code. There is no user cases, no clues about possible evolution.

I chose a factory pattern for the shape creation. To do so it will use creator classes. It is not clear to me which class should be responsible for registering the said creators to the factory. What's usually done in this kind of situation?

enter image description here


Generally speaking, whenever you have two pieces of code that should be decoupled but should work together, you wire them up in a third piece of code that depends on both (assuming you're not relying on some framework or language-supported mechanism that can leverage convention-based naming, or something like that).

In your design DrawingController already depends on both, so you can do it there. If you feel that's an extra responsibility that doesn't belong in that class, you can do it in Program, or introduce another class that will encapsulate the wire up logic (note that Program would then reference this extra class). The code in Program can then obtain the factory from this class, and pass it on to the DrawingController. Also note that in approaches that use dependency injection, most of the hook-up code is pushed to the composition root (essentialy, to main(), see (1), (2)), where you either manually wire up all the dependencies or use a DI container (not saying that one is better then the other).

A few more comments. Another option for shape creation is to use the Prototype pattern. Basically, you maintain a list of AShape instances (of the concrete subclasses), and you just create a copy whenever you need the user add a shape to the drawing. In this approach, the code that "creates" the shape only needs to know how to make a copy (or call something that can give it a copy). This is especially good if the user can create their own shapes - you just add an instance of the custom (user-defined) shape to the prototype list (and you can even do it all at runtime). Another thing you could do is to use the Composite pattern to allow for composite shapes (inherit from AShape, and maintain a collection of AShape-s internally; on draw(), call draw() on each). BTW, composite shapes are another way to support user-defined shapes, and you can also combine this with the Prototype pattern as described above.

BTW, why does your Shape class has the shapeType : ShapeEnum field? Maybe you need to be able to manipulate the vertices, so you cast the shape to the concrete type at some point? If you are using that field in an if-condition somewhere, that could mean that the way you modeled the system doesn't quite line up with what you need the system to do (except if you are only using the ShapeEnum in the factory), so I'd go through the design to see if it's really supporting your needs. (It doesn't have to be "perfect", but consider what are the main characteristics you want the system to have, and make sure that the design actually does something towards that end.) It is also worthwhile to consider if it would help if you found a different set of abstractions (e.g., shapes could be made out of a few component objects that would handle different responsibilities), or even if you changed the granularity of the object-orientation within the system (e.g. make it OO only in the broad strokes, and more procedural and data-oriented in the code that handles drawing). Don't overcomplicate things, though. Also, if I'm not wrong, your application seems to be still at a stage where you can pretty much rewrite everything if need be, so whichever way you go, don't get too attached to the design you've chosen at this stage, and don't try to make something overly general and "future-proof". As time passes you'll come to understand the problem area & the requirements better, and with that knowledge you'll come up with a more suitable design, one that is going to be better than anything you can dream up at this point.

  • Thank you for your thorough answer. I appreciate you made these suggestions too. The shapeType : ShapeEnum is actually useless here, that's a mistake. – Stud Oct 28 '18 at 9:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.