[improvement] Code source improvement

[dbouron]

Hi everyone,

I post an issue in relation to this post on the mailing list in order to discuss about some features.

On my branch, I modernized a little bit the 'services' module source code to make it more readable for contributors. Here is the changelog:

• Remove useless enum/class prefix/suffix with namespace and scoped enum.
• Restructure source code architecture: dispatch class per file.
• Improve IS_PROTOCOL_XX macro with a templated struct using SFINAE concept. Avoiding hand macro insertion/deletion.
• Add FlagTest using google test api.
• Use smart pointer.
• Many other little C++ good practice (C++ cast, loop, auto variable, ...)

I would like to discuss about eventual recommendation from the community, before I continue my work on the others modules. Actually, only service module compile so please just run:

make -C services/

I wrote many point on the TODO file, I specially would like to debate on the following point:

• Usage of Boost.Asio for network layer?
• Document source code using Doxygen?
• Usage of Boost.Filesystem removing dependancy of old tempfile.c?

Thank you in advance for your answers :)

[ghost]

A quick reading says this is better.

I do question if we should use c++14 yet. The standard is only 2 years old and not available everywhere yet. Can we get by with just c++11?

Henry Miller hank@millerfarm.com

On Wed, Oct 26, 2016, at 10:51 AM, dbouron wrote:

Hi everyone, I post an issue in relation to this post on the mailing list[1] in order to discuss about some features. On my branch[2], I modernized a little bit the 'services' module source code to make it more readable for contributors. Here is the changelog:

• Remove useless enum/class prefix/suffix with namespace and scoped enum.
• Restructure source code architecture: dispatch class per file.
• Improve IS_PROTOCOL_XX macro with a templated struct using SFINAE concept. Avoiding hand macro insertion/deletion.
• Add FlagTest using google test api.
• Use smart pointer.
• Many other little C++ good practice (C++ cast, loop, auto variable, ...) I would like to discuss about eventual recommendation from the community, before I continue my work on the others modules. Actually, only service module compile so please just run: make -C services/ I wrote many point on the TODO file, I specially would like to debate on the following point:
• Usage of Boost.Asio for network layer?
• Document source code using Doxygen?
• Usage of Boost.Filesystem removing dependancy of old tempfile.c[3]? Thank you in advance for your answers :) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub[4], or mute the thread[5].

Links:

1. https://groups.google.com/forum/#!topic/icecream-users/pvuhoLv1MgU
2. https://github.com/dbouron/icecream/tree/improvement
3. https://github.com/icecc/icecream/blob/master/services/tempfile.c
4. https://github.com/icecc/icecream/issues/172
5. https://github.com/notifications/unsubscribe-auth/ADPDnD8GXrsxAMUwzuKzrAN7VxU6seCTks5q33cSgaJpZM4KhWDv

[dbouron]

Indeed, there is enough features in C++ 11 to modernize source code. On the other hand, C++ 14 provides some interesting features:

• Variable templates
• Generic lambdas
• constexpr initializer_list
• constexpr function improvements (conditional and loop statements, local variable decalaration, ...)
• type_traits improvements

These could be useful for others modules. I didn't found strong example in services module, but if we need them, we can reconsider C++ 14 usage.

[jdrouhard]

If we are going to overhaul the code and modernize it, there is no reason to shy away from the newest standard.

Feel free to use anything through c++14, but let's try to stay away from a dependency on boost runtime libraries. Header only (build time only) dependencies are fine with me though. We don't want to require boost as a dependency for users when they want to install and use icecream.

On Sun, Oct 30, 2016 at 7:30 AM dbouron notifications@github.com wrote:

Indeed, there is enough features in C++ 11 to modernize source code. On the other hand, C++ 14 provides some interesting features:

• Variable templates
• Generic lambdas
• constexpr initializer_list
• constexpr function improvements (conditional and loop statements, local variable decalaration, ...)
• type_traits improvements

These could be useful for others modules. I didn't found strong example in services module, but if we need them, we can reconsider C++ 14 usage.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/icecc/icecream/issues/172#issuecomment-257148321, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYGi_cDwQ3IXZGCxeQW_KQR55Sn0iLUks5q5I3QgaJpZM4KhWDv .

[dbouron]

I see, maybe we can download boost headers at configuration time like LibreOffice, using wget. By the way, I already use Google test framework. In my humble opinion, we should allow the users to either download this test library at configuration time or decide to skip this framework and launch tests which don't require Google test.

[mich181189]

Maybe we can download boost headers at configuration time like LibreOffice, using wget.

Just a hint, that will make distro maintainers angry ;-)

I'm not sure what the problem with having a runtime dependency on Boost is, I mean, pretty much all distros have packages for it.

That said, I think there's definitely a lot that can be done with boost header-only libraries, and if we're willing to allow c++11 then we can even use some newer STL features to make it better. I'll have a play to see what I can do.

[HenryMiller1]

I think until we release 1.1 we should stick with C++98 and no boost. Once 1.1 is out the door I think we should make C++14 mandatory and take advantage of it.

I'm looking at ASIO as a replacement for all our handwritten networking code (we need to rework the network layer to support ipv6 - #134). I don't see anything else in boost gives us any benefit over what is in C++14, but I'm open to whatever someone comes up with.

[mich181189]

Ok, I think makes sense.

[HenryMiller1]

1.1 is released. Unless someone reports something quick that means we need to release 1.2 in a hurry I think we are good to go with modernization.

The branch in the first message is too large to take in one pull request (it doesn't merge cleanly anyway), but I encourage breaking it up into small requests and doing pull requests on the small pieces.

[AlexanderLanin]

What happened to this cleanup branch? Is it totally outdated by now?

[llunak]

Apparently. But given that, if you feel like taking it over, feel free to do as the above comment says and submit it in smaller pieces (I disagree with some of the changes such as the matter-of-taste renames, and I think it's generally a bad idea to start contributing to a project by dropping a code bomb).

[deriamis]

I'm going to be taking on a series of improvements to the codebase next month, now that I am taking a break from school and the home situation is finally starting to balance out. I'm going to be working off the suggestions in this ticket and merging changes into master in small pieces so we can maintain our ability to do incremental improvements in releases.

Among the changes I'll be making is a bump to C++17, so if anyone has suggestions supported under the new standard, please feel free to make them here. I plan for this issue to remain open until we feel reasonably comfortable that the codebase is in a more widely maintainable state.