3

Suppose that I have two services Person Service and Company Service and I want to maintain links between them for example a Person is linked to Company because he works there or he owns the company etc. So I will go ahead and create a database table like

PersonId, CompanyId, RelationType

Now the business logic can be written in either of the services that client will call to link them. But what if I have requirement to link multiple person to a single company and multiple companies to a single person. I need to have two methods, one that takes single PersonId and list of CompanyIds and one that takes single CompanyId and multiple PersonIds. So I have written two different methods in each service. Following method is in Person Service

void LinkPersonToCompanies(long personId,
                           IEnumerable<long> companyIds,
                           RelationType type)

and below method is in Company Service

void LinkCompanyToPersons(long companyId,
                          IEnumerable<long> personIds,
                          RelationType type)

I know that these are two different methods but they are doing the same task i.e. their logic repeats itself. It can become tough to maintain because a single change in linking mechanism should now be made to both methods. Is it against DRY principle? What should be the right design to efficiently solve this problem?

  • Do the services write directly to the database or do you have intermediate Person and Company objects? – guillaume31 Feb 15 '18 at 9:09
  • @guillaume31 Services write to the database using the Repository pattern. – Saad Feb 15 '18 at 9:12
  • But the Repository pattern uses domain objects. Do you have any? – guillaume31 Feb 15 '18 at 9:24
  • I use EF6 POCO entities for Person and Company, which works fine for my use case. – Saad Feb 15 '18 at 9:46
  • And so Person has a Companies collection and Company has a Persons collection? Or something else? This is an extremely important detail. – guillaume31 Feb 15 '18 at 9:53
6

I know that these are two different methods but they are doing the same task

No they aren't. One is linking a person to a company. The other links a company to a person. They may use the same implementation to achieve these two tasks, but they are not the same task.

Let's assume though that the code in each method is identical. Is DRY violated? Yes, because you have repeated yourself and you have created two copies of the code that need to be maintained. But that may not matter. After all, if you pull the code out into a third method and have the first two call it, you have now created tight coupling between the original two methods and that third one. To clarify here, I'm saying that each of the two original methods are now coupled to the third one; they aren't coupled to each other though. Is it really worth creating that coupling just to avoid repeating yourself?

DRY is the enemy of loose coupling. Sometimes it's worth repeating yourself to avoid coupling. Sometimes it's worth creating tight coupling to avoid repeating yourself. Use your judgement to decide which is the better option in your case.

  • 1
    Good insights. Other than the tight coupling, there are additional drawbacks if I implement a third method as it will take single PersonId and single CompanyId, hence single link will be persisted to database. Multiple linking operation will not stay Atomic in this case. – Saad Feb 15 '18 at 9:10
  • 2
    I disagree that two methods calling the same helper introduces tight coupling between them. Tight coupling implies that any change to one component requires changes to the other, but if both methods are just calls to a helper, any change probably means replacing that call with something else, which automatically breaks the coupling anyway. (Any change to the helper would affect both methods, but that just means that the both methods are tightly coupled with the helper, which is different.) – Sebastian Redl Feb 15 '18 at 9:28
  • 1
    @SebastianRedl, Just to clarify, if Foo and Bar both call Baz, I'm saying that Bar and Baz are coupled and Foo and Baz are coupled. There is no coupling between Foo and Bar. Is my answer unclear in this regard, as you appear to be disagreeing with something I haven't said? – David Arno Feb 15 '18 at 10:30
  • @DavidArno I believe I interpreted the "After all, if you pull the code" sentence as you saying that coupling between all three methods is introduced, including between the two original methods. – Sebastian Redl Feb 15 '18 at 10:47
3

If you didn't have the relationship type you could refactor this to

LinkCompaniesToPersons)(
    IEnumerable<string> companyIds, 
    IEnumerable<string> personIds
    );

and save yourself some code.

With the type in there, its a bit more than a many to many relationship. You could have:

LinkCompaniesToPersons)(
    IEnumerable<CompanyPersonTypeRelationShip>
    );

Which is more generic, but adds code at every call site.

I would go with a combined approach. Have my second method as a private method on the repository and call it via your current easy to read public methods. Then you save code repetition in both the repository and the call sites.

  • Your first method could potentially mean that link each CompanyId to all PersonIds and vice versa. Unless I implement it with one to one correspondence. But your second method takes the link object itself and saves this confusion. Do you think exposing such a method to client is better idea? (Assuming client can be modified) – Saad Feb 15 '18 at 10:07
  • Extending on your idea, you could have two public methods which simply call the common LinkCompaniesToPersons converting the single company id or single person id to a list as necessary. LinkCompaniesToPersons would do nothing but perform a nested loop calling a LinkCompanyToPerson method for an individual company and person. No repetition and you still get pleasant public methods for calling it with. – Neil Feb 15 '18 at 12:32
  • isnt that exactly what I suggest? – Ewan Feb 15 '18 at 12:33
0

Does having these as two separate functions increase readability (of the client code, i.e. is the code that calls them more readable if you leave them "duplicated")? If yes, I say keep them separate.

You can always add a throw (private) function that will perform the actual logic and have these two functions delegate to it - then you won't have duplication but without decreasing readability of the code that uses your interface.

0

It depends on your implementation of LinkPersonToCompanies and LinkCompanyToPersons. As it is both functions have slightly different jobs - one links multiple persons to a company, and the other links multiple companies to a person.

If you reuse the same code (as replicating the logic or shudder just copy-pasting the code) - then this is a violation of the DRY principle.

If you, however have them delegate the task (to a common function, which might even be one of the two functions itself) then it is not a violation of the DRY principle, as there are two distinct tasks (as said before) but you kept the common things, well, common.

For example LinkCompanyToPersons could call LinkPersonToCompanies once for each person (or vice versa), or both could call LinkPersonAndCompany once for each person or company (depending on the calling function).

0

What should be the right design to efficiently solve this problem?

Try to avoid bi-directional associations.

You mentioned in the comments that you have a Person object with a Companies collection and a Company with a Persons collection. Even if there are two use cases, with one screen that allows to add a company to the list of a person's companies and one screen to add a person to the list of a company's persons, that doesn't mean your persistent objects should reflect that.

Bi-directional associations can cause synchronization problems. If a Company and a related Person are loaded at the same time in memory as part of the same unit of work, you have to remember to update both sides as soon as their relationship changes.

Turning this into a one-directional association not only alleviates that issue but also forces you to choose which entity is primary and which is secondary. It makes the code much simpler to reason about because you know there's a single entry point to changes in the relationship, and only it needs to be loaded up in memory. Only one LinkXtoY service method needs to be implemented and you don't have any code duplication issues.

  • You may have missed the part where the OP wrote he creates a link table, which is effectively resolving the problem of the bidirectional association by two unidirectional ones. Still the question about DRY remains the same. – Doc Brown Feb 15 '18 at 14:34
  • @DocBrown I did read that part, thank you very much. I'm not talking about database problems but ones with the object model. The question of DRY cannot be answered until the OP gives the actual body of the methods. I'm providing an answer that alleviates the problem of DRY altogether. – guillaume31 Feb 15 '18 at 14:54
  • (I mean, eliminates) – guillaume31 Feb 15 '18 at 15:10

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.