Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use Boost libraries #127

Open
benwaffle opened this issue Apr 5, 2015 · 19 comments
Open

use Boost libraries #127

benwaffle opened this issue Apr 5, 2015 · 19 comments

Comments

@benwaffle
Copy link
Member

parts of boost we might use:

  • Filesystem
  • Program Options
  • Log
  • Test (if we ever write unit tests)

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

@benwaffle benwaffle changed the title use Boost.Filesystem use Boost libraries Apr 5, 2015
@nyanpasu
Copy link
Member

nyanpasu commented Apr 5, 2015

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
Copy link
Member Author

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
Copy link
Contributor

IGI-111 commented Apr 5, 2015

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
Copy link
Member

nyanpasu commented Apr 5, 2015

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
Copy link
Member Author

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
Copy link
Contributor

IGI-111 commented Apr 5, 2015

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
Copy link
Member Author

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
Copy link
Contributor

IGI-111 commented Apr 5, 2015

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
Copy link
Member Author

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

@IGI-111
Copy link
Contributor

IGI-111 commented Apr 5, 2015

It uses its own types, unlike boost does internally.

@ghost
Copy link

ghost commented Jul 10, 2015

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
Copy link
Member Author

thank you for your contribution

@ghost
Copy link

ghost commented Jul 10, 2015

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
Copy link
Member Author

the Platform_*.cpp files

@benwaffle
Copy link
Member Author

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

@ghost
Copy link

ghost commented Jul 10, 2015

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

@benwaffle
Copy link
Member Author

@ghost
Copy link

ghost commented Jul 10, 2015

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

@benwaffle
Copy link
Member Author

wow rude

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants