Implicitly changing the target file document when saving with a different filename

[rstm-sf]

Implicitly changing the loaded file document when saving with a different filename. I think changing the target document should not change the current WordDocument object. This behavior was introduced due to a document cloning issue https://github.com/OfficeDev/Open-XML-SDK/issues/1139

We can see this

https://github.com/EvotecIT/OfficeIMO/blob/e965a042489a79aa04f630aaf6bb186a3b8284c8/OfficeIMO.Word/WordDocument.cs#L481

https://github.com/EvotecIT/OfficeIMO/blob/e965a042489a79aa04f630aaf6bb186a3b8284c8/OfficeIMO.Word/WordDocument.cs#L588-L599

[PrzemyslawKlys]

@rstm-sf Could we implement a workaround that if "Load" is used a copy to temp first is made so that we don't destroy the source until Open-XML-SDK gets a fix? Not sure if the same applies to loading from a stream. @byteSamurai any thoughts on this?

As @ThomasBarnekow pointed out, this is how it was designed and would indeed take a much deeper architectural change to alter.

I agree, however, that this API is wrongly named. I'm all for suggestions and we can obsolete this one with a warning and provide an API that is better named.

Thoughts of the top of my head:

• SaveAndCopyTo(...)
• SaveAndClone(....)

Other ideas?

Seems this is not going to get fixed anytime soon.

The suggestion is to use:

Yes, SaveAs does save the original. Unless deeper architectural changes are made, it must work that way. The main purpose is basically to create a copy, or clone, of the package at the given path, e.g., even if the package was opened on a MemoryStream. Assuming you want to keep the original file unchanged, you'd have to read all bytes from that file, copy those to a MemoryStream, and later save that MemoryStream using SaveAs.

Would this work for us?

[byteSamurai]

I think changing the target document should not change the current WordDocument object

I see it the same. Apart from a few exceptions, in the best cases, objects should be considered as immutable to avoid side effects from untracked references.

Well, to do it the right thing would require some refactoring: No matter if you follow SOLID, CUPID or GRASP principles: One Class should to just one thing (most of the time).

That would mean: WordDocument is an aggregator for the parts in document.xml. A dedicated class should deal with every loading/storing of the document. This "PackageService" class should implement an interface or abstract class. The concrete implementation should deal with details like: Where/how to store it. A variant concrete implementation could also deal with #36 .

And indeed, this would require using a proper DI implementation and separating the "operation with data" from the "domain model". The change would be pretty invasive.

Not doing that, would lead to bigger and bigger classes, which will have more side effects in the future. So basically, the question is: Is it easier to deal with technical debt or to refactor the code base?

(Sorry, for this wiseacres-like answer)

[PrzemyslawKlys]

Right, but since most of the packing is part of Open-XML-SDK it should be fixed there I guess... and in the meantime we could do some workaround so that OfficeIMO behaves as expected. I was thinking on simply - if we are loading file from path, copy it somewhere else, do the magic and depending if user does Save or SaveAs save it into expected place. Not great, but like you said - proper implementation would take time and is probably outside of OfficeIMO (as far as I understand). Unless you propose to do refactoring here - but then my brain will most likely explode and I will not be able to deliver :-p Someone else will have to do the heavy lifting.