search

ironfede / openmcdf

Microsoft Compound File .net component - pure C# - netstandard 2.0
Mozilla Public License 2.0
295 stars 72 forks source link

Rework the property factory to allow stream specific factories, and tweak the logic for deciding whether a property value should be padded. #108

Open Numpsy opened 4 months ago

Numpsy commented 4 months ago

A few experiments based on #99 and the comments in #98 -

Instead of having a single, singleton instance of a property factory, which is accessed in several places, instead pass an instance of the factory around, and decide which factory to use at a higher level.

When creating a property via the factory, pass the identifier of the property into the factory, so that it can make property specific choices in which property type to use when needed (mainly to use VtUnalignedString for a couple of string properties, rather than VtString)

Add an additional property factory for the DocumentSummaryInformation stream which has specific knowledge of a couple of unaligned string properties.

Also added the unit test and extra test file from #99 / #98 as a test case for a file that isn't currently read correctly.

ironfede commented 4 months ago

Many thanks @Numpsy :I don't think that a solution for the underlying issue can be "super-clean" from a code perspective since the issue derives from a tangled specifc in itself. So I think you should go on on with your experiments towards a fully working solution and only after that starting to think about a clean-up refactoring. I'm sorry for being not-active on this thread but I'm currently out of time to better follow all discussions and evolution of code. Thank you anyway for your developments.

Numpsy commented 4 months ago

I'll have another look now the other changes are merged (I realised whilst testing those that the current code here is wrong, as it tries to use DocumentSummaryInfoPropertyFactory for user defined properties, which doesn't work because you can't determine the behavior based on property identifier for that)

Numpsy commented 4 months ago

Rebased onto the other recent changes, and tweaked a little to minimze the changes (the changes in TypedPropertyValue.cs look bigger than they really are due to bracing / indentation changes)