Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Example contradicts with previous chapters #87

Open
mradecki opened this issue Apr 24, 2016 · 1 comment
Open

Example contradicts with previous chapters #87

mradecki opened this issue Apr 24, 2016 · 1 comment

Comments

@mradecki
Copy link
Contributor

mradecki commented Apr 24, 2016

This snippet from 230_Designing_for_composabiity_protocols.md kind of contradicts with the rules you have described before.

try
{
  gates.CloseAll();
  sirens.TurnOn();
  specialForces.NotifyWith(Priority.High);
} 
catch(SecurityFailure failure)
{
  powerSystem.TurnOffBecauseOf(failure);
}

If I have understood previous chapters correctly, you should use something more general here, like taskForce.NotifyWith(Priority.High) and make pass the instance of SpecialForces class that implements taskForce interface.

EDIT: I will put the second issue from the same chapter here to avoid creating multiple issues.
This might not be an issue or I don't understand it correctly, but this solution just surprised me.

Radio radio = radioRepository().GetRadio(12);
radio.AddPrimaryUserNameTo(primaryUsersList);

In other place, and different example, when speaking of 'ProductName', you said that methods like ToStringForScreen() and ToStringForReport() make the ProductName know too much about how it is used.

Doesn't AddPrimaryUserNameTo make the Radio object know how it is used?

@grzesiek-galezowski
Copy link
Owner

grzesiek-galezowski commented Apr 28, 2016

Hi, I'll look again at the example.

Using toy examples has a disadvantage that they are stripped of the entire context in which they might make sense. As I understand, the question is: is this the right level of abstraction? As always, it depends. Here, it depends on the domain and what meaning do words such as "special forces" and "task forces" have. Is "special force" something generic or not - this mostly depends on the domain.
The second observation is that the whole implementation can be made more generic, for example, it can use the observer interface with gates, sirens and special forces being implementations of a more generic "AlarmTriggerObserver" or something like that. I'll try to look at this example again today and check whether something can be improved or maybe I'll add some comments that make it more clear that it's a toy example.

As for the second example (again, it's a toy example):

radio.AddPrimaryUserNameTo(primaryUsersList);

This is an example of aggregating variable pattern. I couldn't find it on the network, but I saw it named like this somewhere. The point is that whenever you need to aggregate something from several objects into a list, you can pass a single object (in simplest form - a collection) to all of those instances and each instance is free to add whatever it needs to the object. An instance can either add one name, or not add anything, or add several names.

As for the comparison between ToStringForScreen and AddPrimaryUserNameTo, everything boils down to context-independence. Assuming ProductName is a domain abstraction, putting ToStringForScreen() method inside it kind of makes it responsible for part of the presentation logic, because it's the screen that should know how to format the data. On the other hand, adding a AddPrimaryUserNameTo method on a radio fits the responsibilities of Radio as a domain abstraction. All the Radio is dependent on is the collection and all it knows is that it has to put whatever it considers a name inside that collection. The knowledge of whether a radio has a single user, no user or many users is domain knowledge, not presentation knowledge, thus my view is that the radio should be responsible for maintaining this information and it should encapsulate this information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants