Review Request 120412: Simplify code of constructors of KoStore and its subclasses

Boudewijn Rempt boud at valdyas.org
Thu Oct 2 17:11:01 BST 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120412/#review67819
-----------------------------------------------------------

Ship it!


I checked and it doesn't hit problems with my mvc branch and the new code looks much more sensible. I didn't do load/save tests with a range of apps, though.

- Boudewijn Rempt


On Sept. 28, 2014, 10:31 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120412/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2014, 10:31 p.m.)
> 
> 
> Review request for Calligra, Camilla Boemann and Inge Wallin.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> As promised during the Calligra sprint I am currently working on moving UI-interaction out of KoStore classes, both to support automatic processing using KoStore where no UI is happening but also and mainly for supporting non-QWidget environments. 
> 
> And I was confused by the wired setup in the KoStore (sub)classes:
> * Initialisation of the d-object is done directly, not via the constructor parameters as usual
> * wired virtual init method: not only should virtual methods should not be called from constructors, also is there no real need to have this method virtual here, as the base implementation is explicitely called anyway and never expected to be called by someone else. and it also duplicates a lot of the initialisation done by KoStorePrivate constructor.
> 
> Attached patch simplifies the concepts a little, and will make future PRs for deUIfication of KoStore easier.
> 
> Also removes strange initialisation of members in KoEncryptedStore constructor with pattern "member(Type())", so creating a temp object and then passing it to the member's copy constructor, which seems not intended.
> 
> 
> Diffs
> -----
> 
>   krita/image/tiles3/tests/tiles_test_utils.h 87f7c29 
>   libs/odf/KoDirectoryStore.h 7127e35 
>   libs/odf/KoDirectoryStore.cpp 01decfc 
>   libs/odf/KoEncryptedStore.h 2c453f5 
>   libs/odf/KoEncryptedStore.cpp a9b14df 
>   libs/odf/KoStore.h e269ad8 
>   libs/odf/KoStore.cpp 4479deb 
>   libs/odf/KoStore_p.h aff15e4 
>   libs/odf/KoTarStore.h a5437ac 
>   libs/odf/KoTarStore.cpp 772bcb9 
>   libs/odf/KoZipStore.h eff0fb0 
>   libs/odf/KoZipStore.cpp 88afd16 
> 
> Diff: https://git.reviewboard.kde.org/r/120412/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20141002/d353e03d/attachment.htm>


More information about the calligra-devel mailing list