8

Problem
I recently read a lot about Singletons being bad and how dependency injection (which I understand as "using interfaces") is better. When I implemented part of this with callbacks/interfaces/DI and adhering to the interface segregation principle, I ended up with quite a mess.

The dependencies of a UI parent where basically those of all its children combined, so the further up the hierarchy a UI element was, the more bloated its constructor was.

All the way on top of the UI hierarchy was an Application class, holding the info about the current selection and a reference to a 3d model which needs to reflect changes. The application class was implementing 8 interfaces, and this was only roundabout a fifth of the products (/ interfaces) to come!

I currently work with a singleton holding the current selection and the UI elements having a function to update themselves. This function trickles down the UI tree and UI elements then access the current selection singleton as needed. The code seems cleaner to me this way.

Question
Is a singleton maybe appropriate for this project?
If not, is there a fundamental flaw in my thinking and/or implementation of DI which makes it so cumbersome?

Additional info about the project
Type: Shopping basket for apartments, with bells and whistles
Size: 2 man-months for code and UI
Maintenance: No running updates, but maybe "version 2.0" later
Environment: Using C# in Unity, which uses an Entity Component system

In almost all cases, user interaction triggers several actions. For example, when the user selects an item

  • the UI part showing that item and its description needs to be updated. For this, it also needs to get some info from a 3d model in order to calculate the price.
  • further up the UI, the overall total price needs to be updated
  • a corresponding function in a class on a 3d model needs to be called in order to display the changes there
18
  • DI isn't just about using interfaces, it is the ability to swap out concretions which is especially useful in the unit testing sphere...
    – Robbie Dee
    Commented Apr 26, 2016 at 11:07
  • 1
    Also, I've yet to see a problem where a singleton is the preferred solution. The canonical example people always seem to state is a log writer but even here you may want to write to a number of different logs (error, debug etc).
    – Robbie Dee
    Commented Apr 26, 2016 at 11:13
  • @RobbieDee Yes, DI is about more, but I can not see a situation where using an interface is not DI. And if a singleton is not the preferred solution - which I also assume - then why is the preferred solution so messy? That is my main question, but I wanted to keep it open, as sometimes the general rule does not apply.
    – R. Schmitz
    Commented Apr 26, 2016 at 11:42
  • The interface-concretion arrangement is also a feature of mocking frameworks. They've also been in OO languages for a good few decades...
    – Robbie Dee
    Commented Apr 26, 2016 at 11:45
  • I don't get your point.
    – R. Schmitz
    Commented Apr 26, 2016 at 12:01

2 Answers 2

4

I think the question is a symptom, not a solution.

I recently read a lot about Singletons being bad and how dependency injection (which I understand as "using interfaces") is better. When I implemented part of this with callbacks/interfaces/DI and adhering to the interface segregation principle, I ended up with quite a mess.

A solution looking for a problem; that and misunderstanding it probably is corrupting your design. Have you read this SO question about DI vs Singleton, different concepts or not? I read that as simply wrapping a singleton so the client does not have to deal with a singleton. It.is.just.good.old.encapsulation. I think that's spot on.


the further up the hierarchy a UI element was, the more bloated its constructor was.

Build the smaller bits first then pass them into the constructor of the thing they belong in and pass that bigger thing into the constructor of the next bigger thing...

If you've got complex construction issues, use the factory or builder pattern. Bottom line there is that complex construction pulled into it's own class to keep other classes neat, clean, comprehensible, etc.


The application class was implementing 8 interfaces, and this was only roundabout a fifth of the products (/ interfaces) to come!

This sounds like the absence of extend-ability. Core design is missing and everything is being crammed in from the top. There should be more bottom up construction, composition, and inheritance going on.

I wonder if your design is "top heavy." Sounds like we're trying to make one class be everything or anything that it could be. And the fact that it is a UI class, not a business domain class, that really makes me wonder about separation of concerns.

Revisit your design from the beginning and be sure there is a solid, basic product abstraction that can be built upon to make more complex or different category productions. Then you better have custom collections of these things so you have somewhere to put "collection level" functionality - like that "3rd model" you mention.


... 3d model which needs to reflect changes.

Much of this may fit in custom collection classes. It may also be independent class structure unto itself due to depth and complexity. These two things are not mutually exclusive.

Read about the visitor pattern. It's the idea of having whole chucks of functionality wired abstractly to different types.


Design and DI

90% of all the dependency injection you will ever do is constructor parameter passing. So says the quy who wrote the book. Design your classes well and avoid polluting that thought process with some vague notion about needing to use a DI container-thingy. If you need it, your design will suggest it to you so to speak.


Focus on modeling the apartment shopping domain.

Avoid the Jessica Simpson approach to design: "I totally don't know what that means, but I want it."

The following are just wrong:

  • I'm supposed to use interfaces
  • I'm not supposed to use a singleton
  • I need DI (whatever that is)
  • I'm supposed to use composition, not inheritance
  • I'm supposed to avoid inheritance
  • I need to use patterns
3
  • I tried getting rid of all singletons I used and some of the things you said happened more or less automatically. The rest makes a lot of sense, too and I'm gonna take your advice and read up on the visitor pattern etc. All in all a well written answer, thank you!
    – R. Schmitz
    Commented May 4, 2016 at 9:48
  • Just one point, and I'm not trying to be a help vampire here, but it was kind of part of the original question: The general consensus (and it is based on valid reasons) seems to be that you are indeed not supposed to use a singleton. You write "'I'm not supposed to use a singleton' is wrong" - in which situation would it be appropriate to use one then?
    – R. Schmitz
    Commented May 4, 2016 at 9:56
  • All I'm saying is, "it depends." Too often I see maxims taken quite literally.
    – radarbob
    Commented May 4, 2016 at 14:36
3

"Class heirarchy" is a bit of a red flag. Hypothetical: a web page with 5 widgets. The page is not an ancestor of any of the widgets. It may hold a reference to those widgets. But it is not an ancestor. Instead of class heirarchy, consider using composition. Each of the 5 widgets can be constructed on its own, without reference to the other widgets or the top page. The top page is then constructed with enough information to build a basic page and layout the widget objects(Collection) that are passed to it. The page is responsible for layout, etc, but not for the construction and logic of the widgets.

Once you use composition, DI is your best friend. DI allows you swap out each widget for whatever version or widget type you define in DI. Construction of the widgets is captured in the DI and is separate from the top page. Perhaps even the composition of the Collection can be defined in your DI. The top page performs layout based on the widgets passed to it, as it should. No changes to the top page constructor needed. Top page only needs the logic to perform layout of the widgets and pass info to/from widget based on the defined interfaces.

Instead of passing listeners up and down the chain of composition, inject a collection of listeners to those widgets that need it. Inject the collection into a publisher so they can publish to the collection. Inject the collection in the listeners so they can add themselves to the collection. The collection cuts across all the objects in the chain of composition.

11
  • Sorry, "class hierarchy" was wrong wording here; it actually wasn't class hierarchy, but rather UI hierarchy. The 'widgets' are not subclasses of the application class, but the 'page' holds references to them in order to publish UI updates. It was very testable with this structure, but there was also a lot of overhead especially in the constructors, just passing on a lot of listeners for their (UI) children.
    – R. Schmitz
    Commented Apr 26, 2016 at 14:35
  • @R.Schmitz: Did the overhead matter? Was the UI sluggish, and was the design you described to blame? Commented Apr 26, 2016 at 14:57
  • @R.Schmitz Can you elaborate on "passing on a lot of listeners"? Maybe my experience with DI in C# is low, but something tells me that wouldn't be necessary (at least in the constructor) with proper design.
    – Katana314
    Commented Apr 26, 2016 at 15:14
  • @RobertHarvey No performance loss at all, it's only about readability.
    – R. Schmitz
    Commented Apr 26, 2016 at 15:55
  • @Katana314 For example, when an item is added, the 3dmodel needs to be updated and it does not belong in any of the product categories, so it is referenced in the application class, which listens to items being added/removed/changed. In the end that would pretty much be the same for every product category. Other UI parts also react (and listen), but the message needs to travel to the top because that's where the 3dmodel reference and the actual data (which is then also updated) is.
    – R. Schmitz
    Commented Apr 26, 2016 at 16:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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