Sign in to follow this  
Adyrhan

Just released some small project that I have

Recommended Posts

Adyrhan    216

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
Zaoshi Kaba    8434

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
NightCreature83    5002

// 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
Adyrhan    216

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

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