Sign in to follow this  

Just released some small project that I have

This topic is 1514 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi guys! After a long time since I don't write c++ code I've recently decided to retake it. So, to learn again and refresh what I already knew I have written (well, rewritten) a new version of a simple file packaging tool that I have and I've decided to release it GPL'd.
It's here: https://github.com/16BITBoy/packitup
If you feel like to play with some code, please give it a look and tell me what you think and what could I do with it to improve my C++ skills. Thanks and happy hacking biggrin.png

Edited by Adyrhan

Share this post


Link to post
Share on other sites

This is just my personal opinion and nothing constructive.

 

I hate boost, therefore I think this is a terrible idea to use boost for a library you're going to distribute. One of reasons is, to compile your 10 KB program I need to download 50+ MB boost libraries; that's just silly.

 

I noticed some weird things with your includes:

// you are using
#include <boost/unordered_map.hpp>

// but probably all compilers have unordered_map in Standard C++ Library (SCL)
#include <unordered_map>
// another boost include
#include <boost/filesystem.hpp>

// won't complain about this one, but kind of a tip
// Visual Studio 2013 has this include in SCL, don't know about other compilers
#include <filesystem>
using namespace std::tr2::sys;

Your PIUArchiveImpl::write() method (and others) print to standard output, that's unacceptable, use callbacks if output is possible.

Edited by Zaoshi Kaba

Share this post


Link to post
Share on other sites

// another boost include
#include <boost/filesystem.hpp>

// won't complain about this one, but kind of a tip
// Visual Studio 2013 has this include in SCL, don't know about other compilers
#include <filesystem>
using namespace std::tr2::sys;

Just to mention this but "#include <filesystem>" will probably only work on MSVC2013 as it is part of C++14 and nearly no-one has implemented those features yet let along the STD lib that goes with it.

Share this post


Link to post
Share on other sites

T

 

Adyrhan, you might want to fix that link... it's still got a facebook redirect attached to it.

Ooops I don't know how facebook got to be in that link :/ Now is fixed thanks :)

 

This is just my personal opinion and nothing constructive.

 

I hate boost, therefore I think this is a terrible idea to use boost for a library you're going to distribute. One of reasons is, to compile your 10 KB program I need to download 50+ MB boost libraries; that's just silly.

 

I noticed some weird things with your includes:

// you are using
#include <boost/unordered_map.hpp>

// but probably all compilers have unordered_map in Standard C++ Library (SCL)
#include <unordered_map>
// another boost include
#include <boost/filesystem.hpp>

// won't complain about this one, but kind of a tip
// Visual Studio 2013 has this include in SCL, don't know about other compilers
#include <filesystem>
using namespace std::tr2::sys;

Your PIUArchiveImpl::write() method (and others) print to standard output, that's unacceptable, use callbacks if output is possible.

Thanks for your feedback Zaoshi, I'll take note of it. I know the download of boost is not something very pleasant to do, but since my system compiler is gcc 4.6 is not fully c++11, so for unordered_map and for boost::filesystem and some other more I had to use it. I'm a bit afraid of replacing the system compiler (I'm actually using ubuntu linux 12.04 in my dev computer) so I have to either upgrade my linux distro or find a way to have two compilers to coexist in a way that I don't have to alter the build system of the packages I want to compile to use the second one.

Share this post


Link to post
Share on other sites

This topic is 1514 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this