Modules, external factories, guava optionals strategy, logging tweeks #24
Conversation
|
Not ready to merge yet... |
|
Ok, it's ready as is. |
|
Hi Antony, before pulling, I'd appreciate if you could update the HTML documentation (src/site) with details on how to build the Guava part and how it works. |
|
Heads up, big rewrite coming, don't merge it :) (wrote the documentation too). Not sure if the site still builds the way you'd expect though. |
|
That sounds great Antony. The site needs to build the same way it build today by invoking mvn site, also because this will go live online at www.jemos.co.uk/projects/podam/ |
Useful when you can't, or don't want to modify the classes you're hydrating.
Always carry depth with us.
PodamOptional can use custom factories. Documentation for Optional and external Strategies Supports multiple optionals of different types. Also handy GuavaPodamFactory.
|
Would it be ok to run that in a submodule? |
|
I'm not sure I understand what you mean by that? |
|
It's ok, I moved the site back into the root, so it'll work as normal. In re-prepping my pull for merge, i rebased, and I see there's been an attempt to fix the recursive issue I addressed in my branch by @daivanov. However, my recursive collection test still fails against his fix. We both added the depth pass through. See the test here: astubbs@f5b631c . @daivanov - do you want to try addressing that unit test? Or do you want me to try and merge my fix for it? |
|
Yes, if you look at my test I had to reduce max_depth to 3, while default depth is 10. When one produces Pojo -> List as depth is 10 and number of elements in list is 5 it makes Podam to perform too much work (exponential burst) and on some machines it may lead to stack overflow. However, reducing depth to 3 is not practical as many real POJOs might have this depth. Commit 4101540 should not cause any stack overflow as it has no property loop. Could you please remove commit 310617a? Issue with Pdm4PojoUnitTest was fixed two month ago. |
|
Yes, commit 4101540 does expose a problem as it instantiates an inner class and inner classes have parent class as first constructor parameter. This have to be addressed. |
|
FYI I've split the different topics into separate branches, and will submit pull requests for each so we can discuss in isolation. It won't include the guava support though - I'll probably leave that until we have the rest of the stuff settled - as atm it sits on top of a few things. |
|
Shall I close this? |
|
Yes may as well. I'll close it now. Please do go through the other pull requests, I really am dedicated to getting this wrapped up, and getting as much of the changes in as possible. I'd rather not maintain a fork. |
Let me know what you think, just cleaning it up.