Implementation a bodies of functions in header files

[hkarel]

I ask to to move a implementation of bodies non template functions from hpp-files to cpp-files. Now at assembly of your code there are errors, for example: multiple definition of `rcr::alignmean(cv::Mat, cv::Rect, float, float, float, float)'

[patrikhuber]

It's a header-only library, moving stuff to cpp files is the wrong solution. However there are probably a few missing inline keywords, which will get rid of the multiple definition errors.

[hkarel]

Yes, inline directive will solve the problem, but we'll get a copy of bodies these functions in each compilation unit where included headers. For small projects - it no problem, but for large projects - it significantly increase the build time.

[patrikhuber]

Yes. It's a trade-off every library has to make. A header-only library is significantly easier to use and include in a project, particularly on platforms like Android or in conjunction with emscripten.

[hkarel]

Your position is clear, but I remain of the same opinion: non template functions must be in cpp-modules. Especially as your library not the sami big heder-evil connected to my project :)

[patrikhuber]

I agree with you, there is definitely a counter-argument ;-)

May I ask what you mean with:

Especially as your library not the sami big heder-evil connected to my project :)

Thank you for raising this in any case, I will fix the missing inline.

[hkarel]

May I ask what you mean with:

Especially as your library not the sami big heder-evil connected to my project :)

The SupervisedDescent library depends on Cereal and on Eigen. These libraries are also written in header-library style. And they is much bigger on the volume than SupervisedDescent. Therefore SupervisedDescent very little slows down building of the project in comparison with Cereal and Eigen.

[patrikhuber]

Aah, I see! Thanks for the clarification! Yes, particularly Eigen is not small. But I try to always only include the necessary headers, and avoid including a "global" header that includes everything. Still, I agree with you.

[euklid]

Hi, I am also currently using your library and also observed that there were some inlines missing, such that I could only have one translation unit (.cpp file) because with multiple translation units I would get a linker error because of the one definition rule (ODR). Thus, I inlined all non-template functions (without moving them to a seperate .cpp file) and it seems to be working now. Would you be interested in a PR?

[patrikhuber]

Hi @euklid! Yep, I would be very happy to accept a PR fixing the missing inline keywords.

Thank you very much for your nice comment.

-Patrik