Random Number Generator with Loads of Useless(-ish) Features

Question

For fun, and to refresh my memory on Boost's program_options library, I have written a program with the goal of randomly generating numbers with loads of useless(-ish) features. I have termed it diceroll.

#include <algorithm>
#include <boost/program_options.hpp>
#include <boost/algorithm/string/predicate.hpp>
#include <cmath>
#include <iomanip>
#include <iostream>
#include <limits>
#include <random>
#include <vector>

enum returnID {success = 0, known_err = 1, other_err = 2, zero_err = 3, 
    conflict_err = 4, overd_err = 5, underd_err = 6, exclude_err = 7};

const int ld_prec = std::numeric_limits<long double>::max_digits10;

enum fx_enum {prefix = 0, suffix = 1, contains = 2};
bool fx_vect(const long double & rand, const std::vector<std::string> & fx, const int & state);

int main(int ac, char* av[]) {
    try {
        long long number;
        long double lbound, ubound;
        bool ceil, floor, round, trunc; // mutually exclusive
        int precision;
        std::vector<long double> excluded;
        bool norepeat, stat_min, stat_max, stat_median, stat_avg, bad_random, list, quiet;
        std::vector<std::string> prefix, suffix, contains;
        std::string delim = "\n";

        namespace po = boost::program_options;
        po::options_description desc("Options");
        desc.add_options()
            ("help,h", "produce this help message")
            ("number,n", po::value<long long>(&number)->default_value(1),
                "count of numbers to be generated")
            ("lbound,l", po::value<long double>(&lbound)->default_value(0.0),
                "minimum number (ldouble) to be generated")
            ("ubound,u", po::value<long double>(&ubound)->default_value(1.0),
                "maximum number (ldouble) to be generated")
            ("ceil,c", po::bool_switch(&ceil)->default_value(false),
                "apply ceiling function to numbers")
            ("floor,f", po::bool_switch(&floor)->default_value(false),
                "apply floor function to numbers")
            ("round,r", po::bool_switch(&round)->default_value(false),
                "apply round function to numbers")
            ("trunc,t", po::bool_switch(&trunc)->default_value(false),
                "apply truncation to numbers")
            ("precision,p", po::value<int>(&precision)->default_value(ld_prec), 
                "output precision (not internal precision, cannot be > ldouble precision)")
            ("exclude,e", po::value<std::vector<long double> >(&excluded)->multitoken(), 
                "exclude numbers from being printed, best with --ceil, --floor, --round, or --trunc")
            ("norepeat,x", po::bool_switch(&norepeat)->default_value(false), 
                "exclude repeated numbers from being printed, best with --ceil, --floor, --round, or --trunc")
            ("stat-min", po::bool_switch(&stat_min)->default_value(false), 
                "print the lowest value generated")
            ("stat-max", po::bool_switch(&stat_max)->default_value(false),
                "print the highest value generated")
            ("stat-median", po::bool_switch(&stat_median)->default_value(false),
                "print the median of the values generated")
            ("stat-avg", po::bool_switch(&stat_avg)->default_value(false),
                "print the average of the values generated")
            ("bad-random", po::bool_switch(&bad_random)->default_value(false),
                "use srand(time(NULL)) and rand() for generating random numbers (limited by RAND_MAX)")
            ("prefix", po::value<std::vector<std::string> >(&prefix)->multitoken(),
                "only print when the number begins with string(s)")
            ("suffix", po::value<std::vector<std::string> >(&suffix)->multitoken(),
                "only print when the number ends with string(s)")
            ("contains", po::value<std::vector<std::string> >(&contains)->multitoken(),
                "only print when the number contains string(s)")
            ("list", po::bool_switch(&list)->default_value(false),
                "print numbers in a list with positional numbers prefixed")
            ("delim", po::value<std::string>(&delim),
                "change the delimiter")
            ("quiet", po::bool_switch(&quiet)->default_value(false),
                "disable number output, useful when paired with stats");

        po::variables_map vm;
        po::store(po::parse_command_line(ac, av, desc), vm);
        po::notify(vm);

        if(vm.count("help")) {
            std::cout << desc << '\n';
            return returnID::success;

        } else if(number <= 0) {
            std::cerr << "error: the argument for option '--number' is invalid (n must be >= 1)\n";
            return returnID::zero_err;

        } else if(ceil + floor + round + trunc > 1) {
            std::cerr << "error: --ceil, --floor, --round, and --trunc are mutually exclusive and may only be called once\n";
            return returnID::conflict_err;

        } else if(precision > ld_prec) {
            std::cerr << "error: --precision cannot be greater than the precision for <long double> (" 
                << ld_prec << ")\n";
            return returnID::overd_err;

        } else if(precision <= -1) {
            std::cerr << "error: --precision cannot be less than zero\n";
            return returnID::underd_err;

        } else if(vm.count("exclude") && vm["exclude"].empty()) {
            std::cerr << "error: --exclude was specified without arguments (arguments are separated by spaces)\n";
            return returnID::exclude_err;

        } else {
            std::vector<long double> repeated, generated;

            std::random_device rd;
            std::mt19937 generator(rd());
            std::uniform_real_distribution<long double> dis(lbound, ubound);

            if(bad_random) std::srand(std::time(NULL));

            std::cout.precision(precision);

            long long list_cnt = 1;

            for(long long i = 1; i <= number; i++) {
                if(list) ++list_cnt;
                long double rand = (bad_random) ? lbound + (std::rand() / (RAND_MAX / (ubound - lbound))) : dis(generator);

                if(ceil) rand = std::ceil(rand);
                else if(floor) rand = std::floor(rand);
                else if(round) rand = std::round(rand);
                else if(trunc) rand = std::trunc(rand);

                if(vm.count("exclude") && std::find(excluded.begin(), excluded.end(), rand) != excluded.end())
                    continue;
                else if(norepeat && std::find(repeated.begin(), repeated.end(), rand) != repeated.end())
                    continue;
                else if(vm.count("prefix") && fx_vect(rand, prefix, fx_enum::prefix))
                    continue;
                else if(vm.count("suffix") && fx_vect(rand, suffix, fx_enum::suffix))
                    continue;
                else if(vm.count("contains") && fx_vect(rand, contains, fx_enum::contains))
                    continue;

                if(list && !quiet) std::cout << list_cnt << ".\t";

                if(!quiet) std::cout << std::fixed << rand << delim;

                if(norepeat) repeated.push_back(rand);

                generated.push_back(rand);
            }

            if(delim != "\n" && !quiet) std::cout << '\n';

            if((stat_min || stat_max || stat_median || stat_avg) && !quiet)
                std::cout << '\n';

            if(stat_min)
                std::cout << "min: " << *std::min_element(generated.begin(), generated.end()) << '\n';
            if(stat_max)
                std::cout << "max: " << *std::max_element(generated.begin(), generated.end()) << '\n';
            if(stat_median) {
                std::sort(generated.begin(), generated.end());
                auto median = (generated.size() % 2) ? generated[generated.size() / 2]
                    : (generated[generated.size() / 2 - 1] + generated[generated.size() / 2]) / 2;
                std::cout << "median: " << median << '\n';
            }
            if(stat_avg) {
                long double sum = 0.0;
                for(auto i : generated)
                    sum += i;
                std::cout << "avg: " << sum / generated.size() << '\n';
            }

            return returnID::success;
        }

    } catch(std::exception & e) {
        std::cerr << "error: " << e.what() << '\n';
        return returnID::known_err;

    } catch(...) {
        std::cerr << "error: exception of unknown type!\n";
        return returnID::other_err;
    }
}

bool fx_vect(const long double & rand, const std::vector<std::string> & fx, const int & state) {
    std::ostringstream oss;
    oss << std::fixed << std::setprecision(ld_prec) << rand;
    std::string str_rand = oss.str();
    for(auto i : fx) {
        if((state == fx_enum::prefix && boost::starts_with(str_rand, i))
            || (state == fx_enum::suffix && boost::ends_with(str_rand, i))
            || (state == fx_enum::contains && boost::contains(str_rand, i))) return false;
    }
    return true;
}

Here's a breakdown:

1. The program exits through many paths, so to manage this, I use an enum named returnID. This avoids the use of magic numbers in main().

2. The variable ld_prec is the maximum length of a long double. This is used in the function fx_vect() and in main().

3. As said before, I am using Boost's program_options library. Here is the help text:

./diceroll -h
Options:
  -h [ --help ]                produce this help message
  -n [ --number ] arg (=1)     count of numbers to be generated
  -l [ --lbound ] arg (=0)     minimum number (ldouble) to be generated
  -u [ --ubound ] arg (=1)     maximum number (ldouble) to be generated
  -c [ --ceil ]                apply ceiling function to numbers
  -f [ --floor ]               apply floor function to numbers
  -r [ --round ]               apply round function to numbers
  -t [ --trunc ]               apply truncation to numbers
  -p [ --precision ] arg (=21) output precision (not internal precision, cannot
                               be > ldouble precision)
  -e [ --exclude ] arg         exclude numbers from being printed, best with 
                               --ceil, --floor, --round, or --trunc
  -x [ --norepeat ]            exclude repeated numbers from being printed, 
                               best with --ceil, --floor, --round, or --trunc
  --stat-min                   print the lowest value generated
  --stat-max                   print the highest value generated
  --stat-median                print the median of the values generated
  --stat-avg                   print the average of the values generated
  --bad-random                 use srand(time(NULL)) and rand() for generating 
                               random numbers (limited by RAND_MAX)
  --prefix arg                 only print when the number begins with string(s)
  --suffix arg                 only print when the number ends with string(s)
  --contains arg               only print when the number contains string(s)
  --list                       print numbers in a list with positional numbers 
                               prefixed
  --delim arg                  change the delimiter
  --quiet                      disable number output, useful when paired with 
                               stats

If you're wondering what a piece of code does, check this help text, I try to explain thing as best as I can. Let me know if any of this help text is unclear.

4. The flags --ceil, --floor, --round, and --trunc are mutually exclusive. I also return an error if the specified --precision is greater than the precision of a long double (likely won't end up good).

5. I generate random numbers using a statistically psudorandom method, but if --bad-random is specified, I use the bad method.

6. For --exclude, --norepeat, --prefix, --suffix, and --contains, their if statements all result in continue, but I split them up into if-else to make it more readable. If it makes sense to condense them, let me know.

7. At the end(-ish) of the code I print out the statistical variables (min, max, median, average).

8. The flags --prefix, --suffix, and --contains use the enum fx_enum to once again avoid magic numbers.

I am sure my breakdown of the code is incomplete, and if something is confusing please let me know (that's part of the reason I post my code projects here, so I get good feedback!).

I would like to improve the code's readability, I have gotten used to reading it, so my perspective's skewed. I am also wondering if there is optimization that can be done, both in the logic, structure, and how I get things done.

Answer 1

First impressions

I enjoyed reading this code. It's logically laid out (even the includes are alphabetical, so I can check at a glance), and I never felt like I was lost in the code.

I do prefer my enum declarations to have one value per line - this can make it much easier to read diffs in future.

Talking of enums, I'm not convinced there's much benefit to returning distinct values from main() for each possible error category - in general, programs communicate the diagnostic through stderr, and only use the return value to indicate conditions useful to a calling program (rather than to a human). For an example, see the EXIT STATUS section of the GNU Make man page.

Separate the argument handling from the real action

This program has (arguably) reached the size where doing everything in main() is starting to become a burden. It's probably time to separate out the reading of arguments into its own function. A preparatory step is to define a type to hold the program arguments; this is specific to this program and not something to be re-used, so let's just call it program_args:

struct program_args {
    long long number;
    long double lbound, ubound;
    bool ceil, floor, round, trunc; // mutually exclusive
    int precision;
    std::vector<long double> excluded;
    bool norepeat, stat_min, stat_max, stat_median, stat_avg, bad_random, list, quiet;
    std::vector<std::string> prefix, suffix, contains;
    std::string delim = "\n";
};

Replace those locals in main() with a single

    program_args args;

and fix up the compiler warnings by adding args. where needed, and we move to the next stage:

returnID parse_args(program_args &args, int argc, char const *const *argv);

We need to ensure that --help can still return successfully, so add a success_help value to returnID; then the start of main() is simply

    program_args args;

    switch (auto result = parse_args(args, ac, av)) {
    case returnID::success: break;
    case returnID::success_help: return 0;
    default: return result;
    }

I had to change a handful of places where we used vm directly, rather than what it writes to args:

        if (!args.excluded.empty() && std::find(args.excluded.begin(), args.excluded.end(), rand) != args.excluded.end())
            continue;
        else if (args.norepeat && std::find(repeated.begin(), repeated.end(), rand) != repeated.end())
            continue;
        else if (!args.prefix.empty() && fx_vect(rand, args.prefix, fx_enum::prefix))
            continue;
        else if (!args.suffix.empty() && fx_vect(rand, args.suffix, fx_enum::suffix))
            continue;
        else if (!args.contains.empty() && fx_vect(rand, args.contains, fx_enum::contains))
            continue;

By separating out the configuration step from the action step, we've made it much easier to adapt the core logic to work in a GUI or as a Web service. A logical progression would be to separate the output step, which would get us almost all the way to that level of re-usability.

We don't need a separate list of repeated values

The vector repeated is either empty (if norepeat is unset) or an exact copy of generated. So we can just use the generated list for remembering previous values. If we need more efficiency when producing large quantities of output, we could maintain a copy, but as a std::set rather than a std::vector.

Should filtered-out values contribute to the count?

Our main loop for (long long i = 1; i <= args.number; i++) will increment i unconditionally, but I believe the user expects to get number results, rather than some amount up to number. Consider instead looping until we have number results:

    while (generated.size() < args.number)

Yes, this could loop infinitely (e.g. with --prefix ++), but that's down to sensible usage.

Also there seems to be a bug with output numbering - we start list_cnt at 1, but increment it before use, so the first label is 2.

Use the standard algorithms

We can total the elements of a vector with std::accumulate():

    if (args.stat_avg) {
        long double sum = std::accumulate(generated.begin(), generated.end(), 0.0);
        std::cout << "avg: " << sum / generated.size() << '\n';
    }

Similarly, std::nth_element() allows us to find the median without doing a full sort (we might need std::max_element of the lower half, too, for an even-sized array). And we can save a second pass over the results when the user wants both min and max by using std::minmax_element().

Should fx_vect use ld_prec?

It's not obvious to a user that --suffix filters using the full precision of the value, rather than the user's output precision. This seems like an oversight to me.

Also, passing the state argument as a reference to const int loses useful documentation (in the form of the type name) and is inefficient - primitives should normally be passed by value. In this case, state is just used to switch between three functions, so we can just pass the function instead.

I don't really like the name; fx_vect() means little to me. It's really a filter(), as far as I can see.

Extract the selection of good or bad random source

As it is, the bad_random logic is tested outside the loop and at every iteration. We can abstract that into a function that returns our random source wrapped in a std::function:

std::function<long double()>make_random_source(const program_args& args)
{
    if (args.bad_random) {
        std::srand(std::time(nullptr));
        const auto min = args.lbound;
        const auto scale = (args.ubound - args.lbound) / RAND_MAX;
        return [min,scale]{ return min + (std::rand() * scale); };
    } else {
        std::mt19937 generator{(std::random_device())()};
        std::uniform_real_distribution<long double> dis{args.lbound, args.ubound};
        return [dis,generator]() mutable -> auto { return dis(generator); };
    }
}

Then we only need to obtain it (before the loop):

    auto const random = make_random_source(args);

and use it (inside the loop):

        long double rand = random();

---

My version:

This incorporates some additional small changes not otherwise mentioned, such as allowing po::value() to deduce its template type.

#include <algorithm>
#include <boost/program_options.hpp>
#include <boost/algorithm/string/predicate.hpp>
#include <cmath>
#include <iomanip>
#include <iostream>
#include <limits>
#include <random>
#include <vector>

enum returnID {
    success = 0,
    known_err = 1,
    other_err = 2,
    zero_err = 3,
    conflict_err = 4,
    overd_err = 5,
    underd_err = 6,
    exclude_err = 7,

    success_help = -1,
};

bool filter(const long double rand,
            int precision,
            const std::vector<std::string>& fx,
            bool(*predicate)(const std::string&, const std::string&)) {
    std::ostringstream oss;
    oss << std::fixed << std::setprecision(precision) << rand;
    auto const str_rand = oss.str();

    return std::none_of(fx.begin(), fx.end(), [&](auto const& s) { return predicate(str_rand, s); });
}


struct program_args {
    long long number;
    long double lbound, ubound;
    bool ceil, floor, round, trunc; // mutually exclusive
    int precision;
    std::vector<long double> excluded;
    bool norepeat, stat_min, stat_max, stat_median, stat_avg, bad_random, list, quiet;
    std::vector<std::string> prefix, suffix, contains;
    std::string delim = "\n";
};

returnID parse_args(program_args &args, int argc, char const *const *argv)
{
    static auto const ld_prec = std::numeric_limits<long double>::max_digits10;

    namespace po = boost::program_options;
    po::options_description desc("Options");
    desc.add_options()
        ("help,h", "produce this help message")
        ("number,n", po::value(&args.number)->default_value(1),
         "count of numbers to be generated")
        ("lbound,l", po::value(&args.lbound)->default_value(0.0),
         "minimum number(ldouble) to be generated")
        ("ubound,u", po::value(&args.ubound)->default_value(1.0),
         "maximum number(ldouble) to be generated")
        ("ceil,c", po::bool_switch(&args.ceil)->default_value(false),
         "apply ceiling function to numbers")
        ("floor,f", po::bool_switch(&args.floor)->default_value(false),
         "apply floor function to numbers")
        ("round,r", po::bool_switch(&args.round)->default_value(false),
         "apply round function to numbers")
        ("trunc,t", po::bool_switch(&args.trunc)->default_value(false),
         "apply truncation to numbers")
        ("precision,p", po::value(&args.precision)->default_value(ld_prec),
         "output precision(not internal precision, cannot be > ldouble precision)")
        ("exclude,e", po::value(&args.excluded)->multitoken(),
         "exclude numbers from being printed, best with --ceil, --floor, --round, or --trunc")
        ("norepeat,x", po::bool_switch(&args.norepeat)->default_value(false),
         "exclude repeated numbers from being printed, best with --ceil, --floor, --round, or --trunc")
        ("stat-min", po::bool_switch(&args.stat_min)->default_value(false),
         "print the lowest value generated")
        ("stat-max", po::bool_switch(&args.stat_max)->default_value(false),
         "print the highest value generated")
        ("stat-median", po::bool_switch(&args.stat_median)->default_value(false),
         "print the median of the values generated")
        ("stat-avg", po::bool_switch(&args.stat_avg)->default_value(false),
         "print the average of the values generated")
        ("bad-random", po::bool_switch(&args.bad_random)->default_value(false),
         "use srand(time(NULL)) and rand() for generating random numbers(limited by RAND_MAX)")
        ("prefix", po::value(&args.prefix)->multitoken(),
         "only print when the number begins with string(s)")
        ("suffix", po::value(&args.suffix)->multitoken(),
         "only print when the number ends with string(s)")
        ("contains", po::value(&args.contains)->multitoken(),
         "only print when the number contains string(s)")
        ("list", po::bool_switch(&args.list)->default_value(false),
         "print numbers in a list with positional numbers prefixed")
        ("delim", po::value(&args.delim),
         "change the delimiter")
        ("quiet", po::bool_switch(&args.quiet)->default_value(false),
         "disable number output, useful when paired with stats");

    po::variables_map vm;
    po::store(po::parse_command_line(argc, argv, desc), vm);
    po::notify(vm);

    if (vm.count("help")) {
        std::cout << desc << '\n';
        return returnID::success_help;

    }
    if (args.number <= 0) {
        std::cerr << "error: the argument for option '--number' is invalid(n must be >= 1)\n";
        return returnID::zero_err;

    }
    if (args.ceil + args.floor + args.round + args.trunc > 1) {
        std::cerr << "error: --ceil, --floor, --round, and --trunc are mutually exclusive and may only be called once\n";
        return returnID::conflict_err;

    }
    if (args.precision > ld_prec) {
        std::cerr << "error: --precision cannot be greater than the precision for <long double> ("
                  << ld_prec << ")\n";
        return returnID::overd_err;

    }
    if (args.precision < 0) {
        std::cerr << "error: --precision cannot be less than zero\n";
        return returnID::underd_err;

    }
    if (vm.count("exclude") && vm["exclude"].empty()) {
        std::cerr << "error: --exclude was specified without arguments(arguments are separated by spaces)\n";
        return returnID::exclude_err;
    }
    return returnID::success;
}


std::function<long double()>make_random_source(const program_args& args)
{
    if (args.bad_random) {
        std::srand(std::time(nullptr));
        const auto min = args.lbound;
        const auto scale = (args.ubound - args.lbound) / RAND_MAX;
        return [min,scale]{ return min + (std::rand() * scale); };
    } else {
        std::mt19937 generator{(std::random_device())()};
        std::uniform_real_distribution<long double> dis{args.lbound, args.ubound};
        return [dis,generator]() mutable -> auto { return dis(generator); };
    }
}


int main(int ac, char* av[]) {
    try {
        program_args args;

        switch (auto result = parse_args(args, ac, av)) {
        case returnID::success: break;
        case returnID::success_help: return 0;
        default: return result;
        }

        std::vector<long double> generated;

        auto const random = make_random_source(args);

        std::cout.precision(args.precision);

        long long list_cnt = 0;

        while (generated.size() < args.number) {
            long double rand = random();

            if (args.ceil) rand = std::ceil(rand);
            else if (args.floor) rand = std::floor(rand);
            else if (args.round) rand = std::round(rand);
            else if (args.trunc) rand = std::trunc(rand);

            if (!args.excluded.empty() && std::find(args.excluded.begin(), args.excluded.end(), rand) != args.excluded.end())
                continue;
            else if (args.norepeat && std::find(generated.begin(), generated.end(), rand) != generated.end())
                continue;
            else if (!args.prefix.empty() && filter(rand, args.precision, args.prefix, boost::starts_with))
                continue;
            else if (!args.suffix.empty() && filter(rand, args.precision, args.suffix, boost::ends_with))
                continue;
            else if (!args.contains.empty() && filter(rand, args.precision, args.contains, boost::contains))
                continue;

            generated.push_back(rand);

            if (!args.quiet) {
                if (args.list)
                    std::cout << ++list_cnt << ".\t";
                std::cout << std::fixed << rand << args.delim;
            }
        }

        if (args.delim != "\n" && !args.quiet) std::cout << '\n';

        if ((args.stat_min || args.stat_max || args.stat_median || args.stat_avg) && !args.quiet)
            std::cout << '\n';

        if (args.stat_min || args.stat_max) {
            auto minmax = std::minmax_element(generated.begin(), generated.end());
            if (args.stat_min)
                std::cout << "min: " << *minmax.first << '\n';
            if (args.stat_max)
                std::cout << "max: " << *minmax.second << '\n';
        }
        if (args.stat_median) {
            auto midpoint = generated.begin() + generated.size() / 2;
            std::nth_element(generated.begin(), midpoint, generated.end());
            auto median = *midpoint;
            if (generated.size() % 2 == 0)
                median = (median + *std::max_element(generated.begin(), midpoint)) / 2;
            std::cout << "median: " << median << '\n';
        }
        if (args.stat_avg) {
            long double sum = std::accumulate(generated.begin(), generated.end(), 0.0);
            std::cout << "avg: " << sum / generated.size() << '\n';
        }

        return returnID::success;

    } catch(std::exception & e) {
        std::cerr << "error: " << e.what() << '\n';
        return returnID::known_err;

    } catch(...) {
        std::cerr << "error: exception of unknown type!\n";
        return returnID::other_err;
    }
}

---

Further suggestions

If we are testing prefix and suffix, we stringify the value for both tests - perhaps we can do that only once?

Does a non-zero precision make any sense if any of the truncation options are in force?

Could you fix boost::program_options so that it rejects negative arguments to options taking unsigned types? Then we get a better error message when we use unsigned long long for number and unsigned int for precision, as we really would like to.

Perhaps some validation of prefix, suffix and contains, rather than accepting any string value?

Additional statistics - sample and population standard deviations could be interesting tests. You will want to take particular care to keep numeric precision - a good introduction is Tony Finch's introduction to Incremental calculation of weighted mean and variance (you don't need to read all the way through to pick up the essential points - and section 3 provides insight into improving the accuracy of calculating the mean; that's useful even without implementing the variance or deviation calculations).