1

I have Document and ExternalDocument classes in my system, where ExternalDocument extends Document. The main distinction is that ExternalDocument holds onto externalDocumentId and externalEventId data in order to correlate with the external system.

Documents may be overwrote calling document.overwrite(a, b, c). When overwriting external documents I want to track the externalEventId that triggered the change and this is where the design falls apart.

According to the LSP I shouldn't strengthen preconditions in document.overwrite. I could implement an document.externalOverwrite operation and throw an exception when document.overwrite is called directly, but that stills violates the LSP.

The language I use doesn't support generics so I can't go for Document<T> either where T defines the override contract parameter.

I could solve the problem by not inheriting from Document at all and use composition instead, but it feels weird given ExternalDocument still is a Document specialization.

Any guidance?

EDIT:

Just to give a little more context, local documents can be overwrote by a local/user process. External documents are a reflection of documents existing in an external system. I want to communicate the fact that we do not have authority over external documents. The state of those documents is updated in response to remote system events and I want to be able to correlate every state change with a corresponding externalEventId.

Note that some local document operations remain valid on the external ones though, like assigning the document, etc. I'm also trying to keep the business logic within the model as much as possible to avoid an anemic domain model.

After thinking a little more about it I think I may have conflated both "overwrite" operations as one although overwriting a local document & external document are actually distinct processes. I think we could make a parallel between this and having multiple kinds of locks: they can all be opened, but all in very different ways that would be hard to generalize.

Therefore, so far the most logical route seems to be splitting the current concrete Document into Document (abstract base class) and LocalDocument. The overwrite operation would be implemented on both LocalDocument and ExternalDocument. Both implementations could leverage an internal overwrite implementation living in the Document abstract class for parts of the process that are similar.

Obviously clients would have to know what type of document they are dealing with in order to process an overwrite.

Any new suggestions in light of those precisions?

Improve this question
26
  • 4
    Can you clarify: " I want to track the externalEventId that triggered the change" ... "I shouldn't strengthen precondition" What exactly does "track" mean, and how does it constitute a strengthening of the preconditions of overwrite? Commented Oct 8, 2020 at 19:07
  • 2
    Also worth noting: the fact that in the "real world" an ExternalDocument is a Document is not in itself sufficient justification to make ExternalDocument a subclass of Document. Consider the famous square/rectangle problem. Commented Oct 8, 2020 at 19:08
  • @Alexander-ReinstateMonica Ok so basically changes to an ExternalDocument are triggered by receiving an event from a remote system in which case I want to track the ID of the event that triggered the change along with the new state. Both overwrite and externalOverwrite are probably distinct operations. However, externalDocument.overwrite shouldn't normally take place as we do not have authority over those documents. Commented Oct 8, 2020 at 19:13
  • I would probably model that by having something like a PurposeForChange (or something like that) interface, which is an input to overwrite. In the general document case, it can be NoReason object (until you have a business requirement to track why those happened, too), and in the ExternalDocument, it can be a different object that contains the id of the external event that triggered the change. Unfortunately without covariant types, you don't have a way of saying that ExternalDocument.overwrite excepts a ExternalDocumentChangeReason instead of a PurposeForChange Commented Oct 8, 2020 at 19:24
  • What about a Document abstract class with StandardDocument and ExternalDocument. The abtract class wouldn't expose any operation but I could still use it in the repository's contract. However, service classes must now cast to explicit document types, but they kinda need to anyway since we deal with those types differently. Commented Oct 8, 2020 at 19:27

5 Answers 5

Reset to default
1

Indeed, LSP says that everywhere you use a Document you should be able to use an ExternalDocument. This is why you cannot strengthen the precondition. As a consequence, externalDocument.overwrite(a, b, c) should be valid whenever document.overwrite(a, b, c). If overwrite means just not to change the externalDocumentId everything is fine (I could overwrite an external document with the content of an internal document that would replace the content of the external document without changing its identity).

So far so good. But you seem to introduce and additional responsibility to track the origin on the top of external documents. This is no longer about extending the concept of Document: It's adding new responsibilities. Objectively, it seems to me that tracking change events could also make sense for internal documents.

For this reason, your ExternalDocument appears to be only in part an extension. The best way to improve the design is tho ensure separation of concerns between the real extension and the additional responsibilities:

  • limit the ExternalDocument to a Document extension that is LSP compliant;
  • use the decorator pattern to add to a Document or an ExternalDocument the responsibility of tracking change events.
Improve this answer
6
  • Ok, but what if it is prohibited to manually (locally) override an ExternalDocument? If that behavior is inherited and prevented we come back to the LSP violation. Commented Oct 8, 2020 at 19:56
  • @plalx one can override a method. But one does not override a class. The decorator pattern does not override a class: it uses composition over inheritance in conjunction to add responsibilities. So I don't fully get your additional question. If there were requirements missing in your question, please add them there :-) Commented Oct 8, 2020 at 20:12
  • I am well aware what the decorator pattern is. What I mean is that decorators must extend Document. If the overwrite operation is defined in Document then we have the same problem with a decorator or a sub-class. I think the only sane solution is not to define the overwrite operation on the base class extended by ExternalDocument. An overwrite is not the same business process as an externalOverwrite in the end... Commented Oct 8, 2020 at 21:17
  • @plalx the problem is not the same for the decorator the decorator adds new functionality. So you don't touch the exisitng (extended) logic of the parent (i.e. overwrite without event). And if you don't touch parent, there is no LSP issue. What you'd do, is to add a an externalOverwrite() (or better, a traceableOverwrite() to make it useful on any kind of documents). Commented Oct 8, 2020 at 22:02
  • @plalx But if you want to make the native overwrite unusable and force the event-based vriant, there there is no need for a decorator, because indeed you'd violate LSP in one way or another. Then just accept it in your design for justified reason, but don't forget that you'll loose a couple of benefits and increase nasty risks Commented Oct 8, 2020 at 22:03
0

One way to solve this using inheritance is to create an abstract base class BaseDocument that has all the functionality your current Document class has.

Both Document and ExternalDocument can now implement their own overwrite method.

Edit: I deleted this answer because the overwrite method is not part of the base class. Therefor it’s not possible to ‘program to an interface’. I undeleted this answer to allow others to leave comments for the OP why this approach is not ideal.

Improve this answer
4
  • Actually, if the OP can find a way to treat Document and ExternalDocument separately for the purposes of overwriting, and the same for other purposes, then it's fine to have a base class or an IDocument interface without the overwrite method, so this could be a feasible solution. Commented Oct 8, 2020 at 22:45
  • @FilipMilovanović I think this is the best solution so far. overwrite and externalOverwrite are two distinct processes which should be modeled as such. If all documents can't be overwrote in the same way then the base class shouldn't implement that operation obviously. If I had to design up-front that's probably what I would have done, but since Document already existed I only thought of problems inheriting Document would cause. Given there are other shared operations inheriting seems to make sense instead of using composition and dispatching most. Commented Oct 9, 2020 at 11:46
  • It's like if we were modeling locks. They may all have common attributes and certain common operations, but they are all opened differently, hence open couldn't be part of a base class. Some would model open(key), others open(combination). The client has to know the type of lock he's dealing with. In that case perhaps composition would make sense like Lock configured with a LockType strategy, but in that case it would have to be generic so that open(T) matches LockType<T> which is not something I can do in my language. Commented Oct 9, 2020 at 11:57
  • We could always model the opening process using a visitor such as lock.open(lockOpenerVisitor), which double-dispatches to lock.openWithKey(key), but ultimately the specific Lock specializations must implement their own open operation or at least a different protocol for opening e.g. lock.insertKey(key); lock.turnKey() -> LockOpened, etc. Commented Oct 9, 2020 at 13:07
0

Are you absolutely sure all local documents can be overwritten?

They might be read-only, be it due to access-control or whatever. In which case, I would expect some error-indication.

Thus, an ExternalDocument objecting when you try would not violate the base contract at all, even if it does so unconditionally.

Improve this answer
8
  • 2
    That's what I though, at first - you can just say that the overwrite method can throw an exception (even though the base class never does that), and LSP violation is gone, when it comes to exceptions. But, if I understood the question well, the strengthened precondition really comes from the fact that, with the external document, you can't call overwrite anymore at will (once you have an instance); you have to obtain an event id first, and somehow plug it into the object - and this is what actually messes with clients that need to use Document polymorphically. Commented Oct 8, 2020 at 22:34
  • Exactly, processing an external document override requires additionnal inputs such as the externalEventId as a correlation ID. Commented Oct 8, 2020 at 23:30
  • @FilipMilovanović That there is a different way to accomplish some action is independent from the inherited way always being denied. The different way is a pure extension, and thus cannot violate LSP. Commented Oct 8, 2020 at 23:34
  • @Deduplicator - I'm not sure if we understood each other, but the problem is the client; if there's no LSP violation, you don't have to change the client at all (e.g., you may find a way to deal with the event id somewhere else, before the documents are accessed by client code, and it would all just work). However, if the OP cannot find a way to make this work without changing client code, this is precisely what indicates an LSP violation (in the design as it now, against witch the client was written). Commented Oct 9, 2020 at 0:19
  • I'm not saying that this cannot be reconceptualized in a way similar to what you're suggesting, or otherwise redesigned. My point was that exception/denial is not the actual problem OP is trying to deal with, it's just something that appeared as a consideration to the OP while trying to work out through the underlying issue (this new document derivative breaking expectations assumed when clients of Document were written; the OP can't find a clean way to fit it into existing structures). Commented Oct 9, 2020 at 0:19
0

There are two different situations: One, you use inheritance so that an "ExternalDocument" can be used wherever a "Document" can be used. In that situation it should have compatible behaviour. A caller should have no need to know whether they have a Document or an ExternalDocument.

There are other situations where a caller knows nothing about "Document". They are not interested one bit how ExternalDocument is implemented, including that it is a subclass of "Document". It stands on its own. You will never ever be using an ExternalDocument in a place where a different subclass of Document can be used. That's rare. If that's the case, and you don't ever substitute one for the other, then there are no violations.

Improve this answer
0

If you want to add extra parameters to a method without changing the method signature, inject a context object into the constructor. ie

ExternalDoc ExternalDoc(IContext context) public override Overwrite(a,b,c) { base.Overwrite(a,b,c) this.trackChange(this.context.ExternalEventId, "Overwritten") }

Prior to calling overwrite, you set the ExternalEventId in the context, which you keep a reference to after passing it into the ExternalDoc constructor

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.