Rework the property factory to allow stream specific factories, and tweak the logic for deciding whether a property value should be padded. - #108
Conversation
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| internal class PropertyFactory | ||
| internal abstract class PropertyFactory | ||
| { | ||
| private static ThreadLocal<PropertyFactory> instance |
There was a problem hiding this comment.
As it stands, the factory class is stateless, so I don't know why it needs to be ThreadLocal? (if not, that change could be broken out into a separate PR to make things simpler here)
Sorry, something went wrong.
|
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) |
Sorry, something went wrong.
…weak the logic for deciding whether a property value should be padded.
|
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) |
Sorry, something went wrong.
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.