patterncppMinor
Enterprise FizzBuzz in C++11
Viewed 0 times
enterprisefizzbuzzstackoverflow
Problem
I've decided to write a C++ FizzBuzz with the focus on some new C++11 features, ridiculous optimizations and unit-testability.
Are there improvement possibilities in those regards?
Are there improvement possibilities in those regards?
template
static void fizzbuzz(std::ostream& out, const T& number, const std::vector>& values)
{
bool found = false;
std::for_each(values.begin(), values.end(),
[&](decltype(*values.begin()) i)
{
if (0 == number % i.second)
{
out
static void filter_fizzbuzz(const T& end, std::vector>& values)
{
// Needs to be sorted as the filtering only looks for divisible numbers up to the current one
std::sort(values.begin(), values.end());
for (auto i = values.begin(); i != values.end();)
{
// i > end will never become 0
if (i->second > end)
{
i = values.erase(i);
continue;
}
// Is the current number already handled by a divisor?
bool alreadyHandled = false;
for (auto j = values.begin(); j != i; j++)
{
if (0 == i->second % j->second)
{
alreadyHandled = true;
}
}
if (!alreadyHandled)
{
i++;
continue;
}
i = values.erase(i);
}
}
template
void fizzbuzz(std::ostream& out, const T& start, const T& end, std::vector>& values)
{
if (start > end)
throw std::out_of_range("start > end");
if (!values.size())
return;
filter_fizzbuzz(end, values);
for (T i = start; i < end; i++)
{
fizzbuzz(out, i, values);
out << std::endl;
}
}Solution
Know when to use which feature
C++11 offers range based for loops so you should use them instead of
As you can see this facilitates
Don't overuse static
Why are all the functions declared
If so, why?
Design: Unnecessary complexity
I have troubles tracing the algorithm in your code.
If you would really want an enterprise solution I would try to integrate with standards like iterators and function(/functor) interfaces that can be passed into algorithms like
C++11 offers range based for loops so you should use them instead of
for_each with lambdas:for(auto i: values) {
if (0 == number % i.second) {
out << i.first.c_str();
found = true;
}
}As you can see this facilitates
auto instead of the ugly decltype hack.Don't overuse static
Why are all the functions declared
static? Are they part of a class?If so, why?
Design: Unnecessary complexity
I have troubles tracing the algorithm in your code.
If you would really want an enterprise solution I would try to integrate with standards like iterators and function(/functor) interfaces that can be passed into algorithms like
std::transform.Code Snippets
for(auto i: values) {
if (0 == number % i.second) {
out << i.first.c_str();
found = true;
}
}Context
StackExchange Code Review Q#56886, answer score: 6
Revisions (0)
No revisions yet.