javaoopcontrollercohesion

How to break a high cohesive model into different classes


I have a container in which 8 relais live. Now i want to let the client turn on/off different relais. So far so good.

One solution could be:

container.tryTurnOn(1, true);
container.turnRandomOn(true); //turn relais random to on or not.

The 1 means relais 1, and the true means try turn it on; false would mean try turn it off. If i would add there some more methods (changing colour, self-destroying or some other creative things^^) related to the relais itself not to the container, the container-interface would growing up.

So i think a good idea would be having for each relais an own class, so that you could do something like this

container.getRelais(1).tryTurnOn(true);
container.turnRandomOn(true); //this stay the same

Now i also have there different rules: Its only allowed that 6 from the 8 relais can be set to on. (The task of the container to ensure this).

So in that case i have a few problems:

The Relais i get from getRelais is anemic, it only holds the relais state, but the functionality is in the container, because only the container knows if tryTurnOn(true) is allowed or not. (Or should i put the behaviour into the relais, accessing the other relais over the container?) The secound problem i have there is a cyclic dependency: The Relais send the wish of turning on to the container, so it must know the container. The container also must know its childs (the relais), to check if there are never turned more relais on than 6. Also i haven't here any more a Tell-Dont-Ask, because the container ask the Relais for their state and based on the result he access directly the state from the relais and change it.

One extension of the question:

Maybe i want to add there some more rules:

How you would model these in a OO-style? The problem-domain looks very cohesive, because all have to know all. Where you would put the behaviour, would you split the behaviour to different components? I don't now how to do this in OO-style, it seems for me that results there anyway in some procedural-code. But thats why i ask^^


Solution

  • For the purposes of this answer, I will talk about lamps, because I find that easier to visualize.


    On and off, tell-don't-ask, and the growing interface

    First of all, the lamp has no say on the matter. To be more precise, we could allow the lamp to turn off, but the lamp would not know if it can be turn on.

    Trying to model on and off in the lamp results in the lamp asking the other lamps for their state. And that is just not a good design. Even if you don't believe in tell-don't-ask, such design would be nightmare to make thread safe without a global lock.

    Instead, let us treat whether lamps are on or off as extrinsic to the lamp. We will have a collection of lamps that are on. Turning on a lamp is adding it to the collection. Turning it off, is removing it from the collection. The collection can have a set max capacity, and there you have tell-don't-ask. You tell the collection to add the lamp, and it knows whether or not it can and how to do it. You don't have to ask how many items it have and decide based on that.


    You can decide if other attributes need to be extrinsic. For example, you can have the color be an attribute of the lamp. Or you can have a color service, that holds a dictionary/map from lamps to colors.

    I will only talk about on and off, you figure out what other attributes might need a similar treatment.


    Crucially, the interface of the container does not have to grow as you add more attributes. Instead you would just add them to the lamp class, or add a new structure for them.


    Admin

    It would not be responsibility of the collection to know if the user is an admin.

    Instead, there must be a request object, which will be associate to an user session. The boundary must map this request to a controller (that is responsibility of a router), and we would have different controllers for the admin and non-admin case.

    However, the collection of lamps that are on is the same. Thus, we are going to split this class in two: first a storage class, with no constraints, and second a view class that is responsible for the checks and it is the only way to access the storage class. Now the controller can request a view configured with a different maximum capacity depending on whether or not we are handling a request from an admin or not.

    Who makes these requests? The view, of course. There must be some user interface, that a user - who might be an admin or not - will use. Furthermore there must be some session management system, because the user must authenticate, otherwise… how would you have roles?

    Please notice I'm arguing for different controllers for different use cases. Not one big fat controller handling everything.


    The sun

    Turns out the sun is an actor. The sun will initiate operations. When the brightness of the sun changes we might have to turn lamps off.

    Regardless of how you model the sun, we need a sun adapter that will handle the change of brightness of the sun. And when that brightness becomes greater than a threshold (95%) that adapter will call a controller to turn off all the lamps.

    Similarly, we should be able to ask the adapter for the last brightness value. I'm not sure controllers should do this, or the router should do this. Regardless… This should result in requesting a different capacity when creating the view of the collection of lamps that are on.

    I hope you can see how you can expand this idea for more complex rules related to the sun.


    The Composition Root, and Creation and Destruction

    We do not want singletons, and we do not want hard-coded lamps.

    There will be a composition root. By default, the composition root is the class holding the entry point of the application. Although, this can be delegated to a different class, when limitation prevent you from writing the entry point (e.g. you are making a plugin for something else). You can think of the composition root as your glue.

    The composition root will create instances of the different classes needed for the initial state of the application. And this can be done by calling factories. And will link these instances together.

    Thus, the composition root will create the first lamps and add them to a container that holds all the lamps. And that container is not singleton, and the lamps are not hard-coded.

    The composition root will also create the collection of the lamps that are on.


    Now, when destroying a lamp. If the lamp was on, we should remove it from the collection of lamps that are on. Except, given that we are talking Java, by default the collection is keeping the lamp alive.

    Before we begin talking about weak and ephemeral references, or anything like that… I remind you that the container and the collection are not singletons.

    This means that there could be multiple of them. And we might be interested to create multiple of them in a single execution for testing purposes. Which also means we could add a lamp to multiple of them. Which means, we should not be thinking of the literal destruction of the object. But the removal from the container.

    Thus, the container will allow you to set a listener that will be called when a lamp is removed. And the composition root will set that listener to also remove the lamp from the collection of lamps that are on.

    Here the container and the collection are completely decoupled, yet linked at runtime by the composition root. And that is what I mean by thinking of the composition root as your glue.


    Self destroying lamps, would, of course, be implemented by removing the lamp from the container. Perhaps on a timer? I don't know.

    A destroyed lamp is a lamp that still is alive, yet it is no longer in the container of the context on which it was created.


    Ease of use

    This is optional, however, you might want to do it to have your desired interface on lamps.

    As stated here, on and off is not an attribute of the lamps. We might want to pretend it is. Which would mean putting a reference to the collection of lamps that are on in the lamps. And that is not only a form cyclic dependency, but more importantly, that means that the lamp has access to turn off other lamps. That would not do.

    Thankfully we have a composition root to glue things together. We can have the lamp expose turning on and off, and allow listeners that will be called when we attempt to turn on or off the lamp.

    The composition root will then add the listeners on creation… Well, we probably should have the composition root instantiate a factory that creates lamps and sets the listeners on creation. These listeners will then talk to the collection of lamps that are on and off.

    This way, the lamps don't have a dependency on the collection. They just don't know the collection exists. The listeners depend on both, but they don't depend on the listener. Similarly the factory would depend on the listener and the lamp class, but they don't depend on the factory. And finally the composition root depends on everything, and nothing depends on the composition root. There is no cyclic dependency there.

    By the way, given the fact that nothing should depend on the composition root, passing the composition root around is a no-no.


    I want to note that having the composition root do dependency injection (or creating factories that do dependency injection), is the original intent of it.

    If you need to work with an external library (which you don't trust, might want to replace, or hits an external system), you would want to create an adapter for that external library with an interface you control, and inject it from the composition root wherever you need it, instead of having the rest of your code depend directly on the external library.

    If you approach dependency injection the other way around, you would always be asking who injects the dependency? And that question will eventually lead you to the entry point of the application.


    On multiple contexts

    You might consider the following optional, depending on your requirements.

    As I said above, there can be multiple contexts, each with a container and a collection of lamps that are on… that also means that if I add a lamp to multiple contexts, the lamp could be on one context and off in another.

    Sadly, we are talking Java. So, the solution I will suggest is to disallow adding a lamp to multiple containers and collections.

    To do that, we will define a context, we will inject the context to the different objects, and then container and collection can check if we are trying to add a lamp of the right context.

    Wait a minute… That context would have a container and a collection… the context is the composition root! Ah, but that would mean that the context has a container that has the context. And the container has lamps that have the context that has the container. Cyclic dependency strikes back!

    Passing the composition root around is a no-no. We just need to identify it. And for that a simple id value will do. We pass around the id, and we compare the id, and that's it.


    Addendum on output

    Let us say you want to render the lamps. To keep it simple, so you will have an UI where there is a widget for each lamp that should be bright or dark depending on whether or not the lamp is on or off. And it might also change color and other stuff.

    I will give you three solutions.

    1. This output is not intended to update automatically. The user can request to see the state of the lamps. Perhaps can poll it to have it update. In this case there will be controller that handles the request from the user, and this controller will have code that reads the lamps and collection of lamps that are on, and creates a response object (which lack behaviour), and give that response object to the UI. It is then responsibility of the UI to present the response object. See also "View Model" and "Presenter".

    2. We want the output to update automatically. But changes are sparse, so we don't want the UI to be working often. You can allow listeners for change on to the collection on the lamps. The composition root would then inject listeners that will (asynchronously, preferably) call the UI to update its view model.

    3. We want the output to update automatically. Changes happen all the time. We care about doing this fast. Have the UI thread read the container and collection directly, once per frame, and render based on that.

    Don't know which to use? Use an adapter.


    Addendum on testing and singletons

    If we want to automate testing, we usually want to run a long list tests on a single execution. For that we do not want tests to affect each other. They should be independent.

    Which often is understood to mean that tests should not have side effect (evidently, if they don't have side effects, they can't affect each other). Which would lead to mocking, et.al. but let us not go there.

    However, if our solution creates a singleton container, and singleton lamps, then the state of these would carry on from one test to another. Leading to the next of extra code to clean up/tear down after each test.

    Add to that all the issues with singletons, such as making it very easy to sneak hard-coded dependencies (instead of using dependency injection), which can make the code harder to reason about and maintain.

    Instead, we do not want them to be singleton. We want each test to create a new container, lamps, etc... We want each singleton to happen in a separate context with separate objects.


    Addendum on Adapter vs Controller

    Both adapters and controller are ways to deal with external systems. If an external system is an agent/user (it can initiate operations on your system), you want controllers to handle the requests from it.

    On the other hand, an adapter is intended to control an external system, in such way that you can replace that external system.

    What if you have an external system that needs both? You make an adapter, and this adapter can call controllers.

    For example. We make a text only user interface, we may want to replace that in the future with some GUI or perhaps make a web version, or something like that. Refactor the code so that all the UI I/O is in one place. Extract that into a class with an interface under your control. That is your UI adapter. You can then implement the same interface differently (for example with a GUI), and that will allow you easily replace the UI. That is a form of polymorphism.

    Of course the UI should allow the user to interact with the system. Thus, this UI adapter should be able to call controllers to do different operations. Turning lamps on and off, and so on. The controller should worry about how to do that.

    Sometimes you need to insert something in the middle that will give a different controller to the adapter depending on the state of the system (whatever or not the user is authenticated, if the role is admin, etc.) - That is a router.


    Is this procedural?

    Arguably, yes. And we probably can say that of any real world OO solution. It is ultimately procedural if you look closely. However, importantly, we are going for tell-don't-ask. Yet, there is a bastion of procedural code in any getter-setter pair (asking and not-really-encapsulation pair), so perhaps there is room for improvement.

    This is OO in the big picture. The thing we need to realize is that not every object has to be an entity. Some objects are services, and have only behavior, and they will be very procedural inside. And some other object, sadly, are values, and have no behavior. For those we probably would not use objects if we were in a different platform (other than Java).

    Please note that values are necessary. That is why we have int, and bool, and so on.

    Think about them as little computers passing messages. Sometimes the messages are too complex for the native types. Then we make a type for those messages. That is meant to be a value… You know, like int, but more complex… But Java is the way, it would have been a struct in some other languages. Also, not every computer in the network needs an state, they can be servers that simply receive some input, process it, and send a message down the line.

    We are not breaking encapsulation. In fact, by hiding the domain model behind controllers, these controller can be modular, and changes do not have to propagate across the controllers (if we want to change how we store the lamps, that does not have to change the UI that calls the controllers. Similarly, if we want to change the UI, we do not need to change the controllers). The controllers provide a layer of abstraction.

    We have made a system that, itself, behave more or less like one big object. The controllers are the methods, the domain model is the state. You can have multiple of this in a computer. In fact, if you use controllers that talk across a network, to other systems designed with similar architecture, you could have multiple computers working like one big computer. Some of them could be dedicated to the state, we call them database servers. Other could be dedicated to process information, they can be application servers (or similar). It is computers with little computers inside, with little computers inside…

    However, once you hit the lower levels, at some point, you need some procedural code.

    Let us see the pillars: Abstraction, check, the controllers do that. Encapsulation, check, although we flip it on its head with on and off, it cannot result in an invalid state (we want to prevent lamps in multiple containers to make this correctly). Inheritance, you don't need it (well, we have interface inheritance). Polymorphism, check, all the dependency injection.

    Let us see the S.O.L.I.D principles: Single responsibility, check, in fact, you seem to complain I break too much, however, the problem with single fat controller is that it has too many reasons to change. Open-closed, check… more or less, I have not proposed any extension points other than add a new controller or adapter. Liskov substitution, check, we are not violating it, although not a textbook example, I'm suggesting you can replace adapters. Interface segregation principle, check? hmm… I haven't covered that, but well, follow it where it makes sense while implementing. Dependency inversion, check, all the way, I'm telling you to not hard-code dependencies, instead inject them with a composition root, also no singletons.


    Addendum on a convoluted toy example

    Perhaps there is something to say about kinds of attribute. For a toy example, let us say we have car objects.

    To begin, let us say cars have models and colors.

    Should we make a class for each model? Perhaps, if the functional differences between one model and another are relevant for the system. Otherwise, we can make an attribute for the model.

    We can ask the same about the color, should we make a class for each color? And you can see how thinking about this would lead to multiple inheritance and its problems. If in the future we want cars with multiple colors, perhaps even drawings, we are better off with a coloring component, an attribute.

    What if we want to talk about the owner of the car? Should that be an attribute of the car? Perhaps. However, the car class does not operate on it. You want the data with the behavior. Ownership is extrinsic to the car. For a database we may put an owner field, that is very data-oriented. However, a OO design would suggest that people have a list of cars they own… Wait, you could put a car as owned by multiple people. Does that make sense in the system? If multiple owners don't make sense, that is an invalid state. A failure of encapsulation.

    We can deal with this by using a dictionary that maps cars to owners, being the car the key, each car has only one owner. To ease getting the list of cars of a person, we make a class that contains the dictionary from cars to owners, but also a dictionary from owners to lists of cars, and it is responsibility of this new class to maintain the invariant that the owner of a car has that car in its list, and viceversa.

    In fact, moving attributes to dictionaries is usually a viable in general. With the drawback of handling destruction. Given that the dictionary holding a reference to the object, prevents garbage collection. Over do this, and make it generic, and you have the core of a Entity-Component-System, which is a data-driven design. Not a violation of encapsulation, but it is backwards. It goes against all object oriented design methodology… Yet, it can be a better solution if you have a lot of interdependent rules that require working around traditional encapsulation.

    What about whether or not the car is on or off? That is intrinsic, right? The behavior is in the car. Until, you give me an invariant that is outside the car, such as the requirement that there could only be six car on. An invariant that cannot be uphold inside the class suggest we need a new class. What class can naturally represent an invariant of a set objects limited by their count? I know, a collection (a set, or a list, or a vector).

    Now we need the car to cooperate with the collection, and we have a cyclic dependency. This answer is my attempt to solve that.


    *By the way, in a prior attempt at this answer, I was going full Model-View-Controller.

    All problems in computer science can be solved by another level of abstraction. … But that usually will create another problem.

    -- Paraphrasing a quote attributed to David Wheeler.