2

Evening. I'm having trouble finding an appropriate design pattern for some situations of deep composition. Let me present an example.

Let's say we have a class of type Corporation that has many classes of type Subsidiary that have many classes of type Department that in type contain many classes of type Unit that in turn contain many classes of type Employee.

Now, suppose the use case is to count the number of employees for each corporation. I could loop through each corpration, loop again for each subsidiary, and so on and so forth, in something that would result in a nested loop, 4 levels deep. Plus, I would be breaking the Law of Demeter by referencing my class chain several levels below, something that is so tightly couped it would break the very moment I modified my chain.

Another thing I could do is add tons (ok maybe not tons, but a few) of shortcut references. For example, a corporation could itself ALSO contain a list of Employees resulting in never having to walk through the chain to count them. This way, classes are less tightly coupled (but are they?) and the issue now becomes how to keep the Employee list synced for both the Corporation and the Unit. I could use the Observer pattern to keep them updated I suppose but I really feel something's horribly wrong with this idea or, at the very least, I'm not really using the best solution out there.

As I'm pretty sure this is an extremely common domain, could anyone be kind enough as to point me to an appropriate design pattern?

Thanks.

ctsag
  • 131
  • 3

2 Answers2

1

I don't exactly get the second question but I am answering the first question.

As the Law of Demeter states that each entity should have least knowledge about other units

So using that principle in your design

class Corporation{

    //All the stuff about corporation

    //Don't ask for what's inside corporation
    public int countEmployees(){
        //will apply the logic needed to calculate
    }

}

Better Client code with Law of Demeter:

corporationInstance.countEmployees(); //let the corporation handle how to count and not expose inner details

Without Law of Demeter

corporationInstace.getSubsidiaries().getSomethingElse()..... //exposing the inner details of class which creates a chain that is bad.

UPDATE:

Using the above stated solution you can go in as many depths as you want by creating the countEmployees() method inside Subsidiaries and in Unit as required. There is no point in breaking the encapsulation or using Observer pattern here.

Apply the Tell Don't ask principle as you have pointed in the comment yourself and delegate the responsibility of calculating the actual employees on the class that contains employees.

Department - > uses count method on subsidiaries to add their count
Subsidiaries - > uses Units to count
Unit - > Uses employees to count
Narendra Pathai
  • 41,187
  • 18
  • 82
  • 120
  • That's what I was suggesting by mentioning shortcut methods. However, if I were to implement a countEmployees(), I'd need to have, say, an array of Employees, right? Which would essentially have to be the same array of Employees the Unit class would contain. And I'd need to keep them synced, probably with an Observer pattern. Is this what you're suggesting too? – ctsag Aug 30 '13 at 20:12
  • No whose property is Employees? Is it of Unit? Then you will need to make shortcut methods for Subsidiaries, which would in-turn call shortcut method of unit which would contain employees. You don't need to extract employees out of the unit class. – Narendra Pathai Aug 30 '13 at 20:17
  • So, each method in a sequential class chain would contain a countEmployees() method that would call the next class' counteEmployees() method. It makes sense I suppose, in that I can count the employees at whatever level of the chain I want. I like this approach, thanks for your input Narendra. By the way, how would you call this principle? Is it [Tell, don't ask](http://pragprog.com/articles/tell-dont-ask)? – ctsag Aug 30 '13 at 20:21
  • I agree with this approach. The corporation might do it's count by asking each subsidiary to do it's count and totaling those results, and so on down the line. But this way, if you have a special case in the minutia, like a director in one subsidiary who is over two units, that subsidiary can be responsible for making sure he is counted exactly once in it's count method. – Mark Bailey Aug 30 '13 at 20:24
  • Yes that is what Law of Demeter states you wont have to pull out employees. Also mind the responsibility principle, employees should be a part of class that actually has responsibility to have them :) – Narendra Pathai Aug 30 '13 at 20:24
  • http://misko.hevery.com/code-reviewers-guide/flaw-digging-into-collaborators/ this a nice post on the same topic – Narendra Pathai Aug 30 '13 at 20:27
  • Yes it is tell don't ask. http://www.captaindebug.com/2012/03/defining-tell-dont-ask-well-almost.html#.UiEAoZJJMyQ – Narendra Pathai Aug 30 '13 at 20:30
  • I have updated the answer incorporating the proper explanation with the link. – Narendra Pathai Aug 30 '13 at 20:37
  • Thanks for the answers guys. I think however that Mark's observation is quite interesting. If class A in the graph delegates its count to the next class, say, (each) class B that means that when class A contains multiple classes of type B, it has no way of knowing whether an employee is accounted for twice (because, as Mark pointed out, an employee could be assigned to two or more units. To do that, class A would have to aquire a set of all employees to eliminate duplicates). So, instead of delegating countEmployees(), it should perhaps delegate listEmployees(), no? – ctsag Aug 30 '13 at 20:51
  • No delegating listEmployees() will break encapsulation. What other things does Unit perform. I am thinking that Unit should be a property of Employee, as an employee can also be a part of multiple units. – Narendra Pathai Aug 30 '13 at 21:04
  • How would listEmployees() break encapsulation? If i were to name it getEmployees() it would simply be a getter for a private property of type Employee[]. So what's wrong with that, encapsulation-wise? The Unit class might do a number of other things, provide costs, statistics, etc, ie, it doesn't have to be a dummy class. Also, the duplicate employee problem doesn't have to be limited to the Unit level, it could go all the way to the top, an employee could be assigned in two different departments too (even subsidiaries if you choose to ignore the legal implications). – ctsag Aug 30 '13 at 21:13
  • How I see this is there can be an HR department who keeps the references of all the employees and knows how many employees exist. Now the call for assigning an employee to a department will also go through the HR department and so on. Now in that case the HR will allocate that employee to that department. This is just my viewpoint of seeing it. It breaks incapsulation as you are passing the whole list to caller and caller can add() remove() anything from it. Now if that is accepted then you can also go with listEmployees(). Rules can be broken too.. :) – Narendra Pathai Aug 30 '13 at 21:19
  • No, I see your point about encapsulation but that could easily be fixed by returning an immutable list. I've read your link to the [Tell, don't ask](http://www.captaindebug.com/2012/03/defining-tell-dont-ask-well-almost.html#.UiEOWxvrwpl) principle and I was wondering why getAllItems() (the equivalent of listEmployees() in our case) is a bad idea. After all, a standard view of a shopping cart doesn't just present totals and sums, it also presents which Items you're purchasing, no? So, if it's not the responsibility of a ShoppingCart object to enumerate the Items, whose responsibility is it? – ctsag Aug 30 '13 at 21:30
  • Yes you can ask for all items in cart, but you can achieve same using toString() for enumerating all items for showing somewhere. That said it is perfectly valid to break the rules as they are just guidelines. Also I would like to state that modelling real world entities in an OO manner is hard. – Narendra Pathai Aug 30 '13 at 21:35
0

Say you want to email the customer from a link or button. You might write it like customer.getSomeParticularContactInfo(addressType).sendEmail() or customer.sendEmail() which then (inside Customer) calls getSomeParticularContactInfo("primary").sendEmail().

You are on the wrong way. This breaks Single Responsibility, I mean, Customer Object doesn't need to know, how can send E-mail, Customer object is responsible only for how to provide the e-mail address belongs to the customer. So for this functionality, you need to create another Interface like Notifier and an EmailNotifier what implements Notifier. Thereafter you will call EmailNotifier.notify(customer)

fureszpeter
  • 120
  • 6