Shared pointer should be used where appropriate

kiwix / libkiwix

Common code base for all Kiwix ports

https://download.kiwix.org/release/libkiwix/

GNU General Public License v3.0

117 stars 55 forks source link

Shared pointer should be used where appropriate #988

Closed kelson42 closed 11 months ago

kelson42 commented 1 year ago

See https://github.com/kiwix/kiwix-android/pull/3467

kelson42 commented 1 year ago

@veloman-yunkan @mgautierfr Reading https://github.com/kiwix/libkiwix/pull/991#pullrequestreview-1596804298, it seems to me there is a discrepency between both of you on (1) the nature of the problem (2) the path to follow to improve the situation. I have no optinion at all, but I would prefer to have you reaching a kind of of agreement on these two points before continuing PR implementation.

mgautierfr commented 1 year ago

Here a proposition:

Move html_dumper, libxml_dumper and opds_dumper behind a free function (implementation will not be changed, but classes will not be part of the public api).
Make server take a shared_ptr to library and namemapper.
Keep the proposition in #991 to create the library only on the heap through a shared_ptr (?)
If previous point is not done, introduce a free function as proposed here to be able to create a not owning shared ptr (internal only ?) for case when user want to create a a server on library

veloman-yunkan commented 1 year ago

Here a proposition:

Move html_dumper, libxml_dumper and opds_dumper behind a free function (implementation will not be changed, but classes will not be part of the public api).

Make server take a shared_ptr to library and namemapper.

Keep the proposition in Use std::shared_ptr instead of raw pointer. #991 to create the library only on the heap through a shared_ptr (?)

If previous point is not done, introduce a free function as proposed here to be able to create a not owning shared ptr (internal only ?) for case when user want to create a a server on library

I am fine with that