gtorrent / gtorrent-core

Core library of gTorrent which handles everything but UI/UX
GNU General Public License v3.0
39 stars 12 forks source link

use Boost libraries #127

Open benwaffle opened 9 years ago

benwaffle commented 9 years ago

parts of boost we might use:

some of this is part of TR2...how do the TRs work?

nyanpasu commented 9 years ago

I'm strongly in favour of unit tests.

Just, I've never liked Boost.

Nothing wrong with writing simple tests without any fancy frameworks. Just that boilerplate and oft inconvenience.

Some promising link(s): http://www.cmake.org/Wiki/CMake/Testing_With_CTest

benwaffle commented 9 years ago

Boost seems nice since it has the functions we use. What's bad about it, exactly? Also, IDK if unit tests are worth the effort. gtorrent-gtk can be our test since literally nobody else will use -core.

IGI-111 commented 9 years ago

I'll be the devil's advocate here then.

It's a bad idea to use boost because most of the functionnality it would provide is either trivial to implement or complicated enough that you'd use a dedicated lib. The main problem with using boost is that once you do, you're stuck with it, since it depends on itself heavily and is more a superset of C++ than anything else, once you have a few includes it's very hard to move away from using it. And that means keeping up to date and linking with a huge, ever changing library, which is a huge pain, not to mention the unreadable compiler output because its namespace tree is deep enough only R'lyeh could be a the bottom.

That and C++ has some awesome unit testing frameworks which don't use boost.

That said, you might want to cover core with tests because that's where the critical stuff happens, but that supposes having the API standardized.

nyanpasu commented 9 years ago

Also, IDK if unit tests are worth the effort. gtorrent-gtk can be our test since literally nobody else will use -core.

Because we totally have a way of directly finding out whether a function or feature in core works without probing it indirectly with a graphical application.

Tests aren't hard to write, either. 100% coverage isn't the goal, and the core lib is simple as fuck now, so I would write tests for new features as they get written.

If you were going to test them yourself you might as well write tests so that it can help everyone see whether it's behaving correctly without having to do some cumbersome dance over a GUI.

Couldn't agree more with everything @IGI-111 brought up about boost.

The only places I would involuntarily use boost is where libtorrent already uses it.

benwaffle commented 9 years ago

It's a bad idea to use boost because most of the functionnality it would provide is either trivial to implement or complicated enough that you'd use a dedicated lib.

I was thinking of using Boost.Filesystem which is non-trivial and should handle errors properly. As for not rewriting settings, do you have an alternative to Boost.Program_Options?

The main problem with using boost is that once you do, you're stuck with it, since it depends on itself heavily

If it depends only on itself you're not stuck with it.

and is more a superset of C++ than anything else

isn't that good?

once you have a few includes it's very hard to move away from using it.

If we do use it there's no reason to switch later.

And that means keeping up to date and linking with a huge, ever changing library,

Isn't it a bunch of separate libraries?

not to mention the unreadable compiler output because its namespace tree is deep enough only R'lyeh could be a the bottom.

You can use clang++. Also, it's pretty hard to fuck up calling a function like exists("~/.config"). I don't suggest we use the hacks like adding lambdas to c++98

IGI-111 commented 9 years ago

do you have an alternative to Boost.Program_Options

getopt() is still the standard way to parse options in C++ and for good reason, it's not broken and it doesn't need fixing.

If it depends only on itself you're not stuck with it. Isn't it a bunch of separate libraries?

You didn't understand what i mean because you haven't had to deal with boost in a big project. Boost is a bunch of "separate" libs relying on each other, once you import one you need the others.

If we do use it there's no reason to switch later.

That's short sighted, and being locked into using something is never a good thing.

You can use clang++.

I use LLVM already thank you very much.

Also, it's pretty hard to fuck up calling a function like exists("~/.config")

You mean boost::filesystem::exists(boost::filesystem::path(getenv("HOME")) / boost::filesystem::path(".config")); ?

benwaffle commented 9 years ago

getopt() is still the standard way to parse options in C++ and for good reason, it's not broken and it doesn't need fixing.

boost.program_options handles config files

You didn't understand what i mean because you haven't had to deal with boost in a big project. Boost is a bunch of "separate" libs relying on each other, once you import one you need the others.

Ok, but why is that bad?

That's short sighted, and being locked into using something is never a good thing.

Why would using boost lock us in anymore than any other library?

You mean boost::filesystem::exists(boost::filesystem::path(getenv("HOME")) / boost::filesystem::path(".config")); ?

can't you add using namespace boost::filesystem;

IGI-111 commented 9 years ago

config files

Well since we want text files and don't want to have to write yet another parser, why is boost relevant here? Might as well use a dedicated parser, that'll be more efficient.

can't you add using namespace boost::filesystem;

Your compiler won't care though, and it conflicts with some other functions so you'd use namespace fs = boost::filesystem; but that is besides the point, it's still much more complicated than just using the STL for virtually no gain.

Let's be clear here, even asio has a boost free version because most people don't want to have to deal with boost stuff when the STL is better and unfortunately since C++ is static and boost libs are written to be extensions of the language, once you import a boost lib, it almost always relies on other boost libs that you might have to import.

In other words boost is a framework more than just a lib and sticking with separate libs and plain C++14 is a million times better than being stuck using boost.

benwaffle commented 9 years ago

libtorrent-rasterbar already uses boost, so would you say we've already imported all of it?

IGI-111 commented 9 years ago

It uses its own types, unlike boost does internally.

ghost commented 9 years ago

I've used boost filesystem, and have even examined the code. It's a bit on the complicated side, but overall, it seems to just be a lib that has abstraction you''ll have to write anyways.

benwaffle commented 9 years ago

thank you for your contribution

ghost commented 9 years ago

After looking through so far, in at least the core, I don't see anything that appears to really be benefited from using boost::filesystem. I'll check the curses and Qt client.

benwaffle commented 9 years ago

the Platform_*.cpp files

benwaffle commented 9 years ago

By the way, the Qt client has a lot of code so don't rush

ghost commented 9 years ago

W-where's the Qt client? ;_;

benwaffle commented 9 years ago

https://github.com/gtorrent/gtorrent-qt

ghost commented 9 years ago

But... I mean, there's no actual client!

benwaffle commented 9 years ago

wow rude