I'm trying to implement a template that takes a container as parameter. The template has a getnext
method that cycles through the elements in the parameter. Take note that I'm not using C++11.
Header:
#pragma once
template<template<class, class> class TContainer, class TObject>
class cycle
{
public:
explicit cycle( TContainer<TObject, std::allocator<TObject>> & container )
: mContainer( container ), index(0) {}
TObject getNext(int numObjectsToCycle)
{ return mContainer[index++ % numObjectsToCycle]; }
private:
TContainer<TObject, std::allocator<TObject>> & mContainer;
int index;
};
Implementation:
#include <iostream>
#include <vector>
#include "cycle.h"
using namespace std;
class Processor
{
int mIndex;
vector<int> numbers;
public:
cycle<vector, int> cycler;
// Is it safe to pass numbers since it is declared first??
Processor() : mIndex(0), cycler(numbers) {}
void update() { cout << numbers[mIndex++ % numbers.size()] << std::endl;}
void addNumber(int x) { numbers.push_back(x); }
};
int main()
{
Processor S;
for (int i = 0; i < 5; ++i)
{
S.addNumber(i+1);
}
cout << "using update" << endl;
for (int c = 0; c < 10; ++c)
{
S.update();
}
cout << "using cycle" << endl;
for (int c = 0; c < 10; ++c)
{
cout << S.cycler.getNext(5) << endl;
}
std::cin.get();
}
Any improvement or potential issues to the code?
3 Answers 3
Here is how I would write it:
template<typename C>
class cycle
{
C& mContainer;
size_t index;
using ref = decltype(mContainer[0]);
public:
explicit cycle(C& container) :
mContainer(container), index(0) {}
ref getNext()
{
return mContainer[index++ % mContainer.size()];
}
};
Notes:
The template parameter is the container,
C
. This not only simplifies code, but also works for containers that do not follow a specific pattern, e.g. have more parameters than just a value type and an allocator type.The return type of
getNext
is automatically inferred fromC
. This is not justC
'svalue_type
but a (non-const
) reference to it.If I understand correctly from the title,
C
is supposed to be a sequence of containers? If so, thevalue_type
is a container itself so it is even more important that a reference is returned fromgetNext
.Why pass a parameter to
getNext
? To cycle correctly, you should usemContainer.size()
, exactly as you do inProcessor::update
, right?index
is of typesize_t
With only a few changes, here is Processor
:
class Processor
{
size_t mIndex;
std::vector<int> numbers;
public:
cycle<std::vector<int>> cycler;
Processor() : mIndex(0), numbers(), cycler(numbers) {}
void update() { std::cout << numbers[mIndex++ % numbers.size()] << std::endl;}
void addNumber(int x) { numbers.push_back(x); }
};
Notes:
- again,
mIndex
is asize_t
cycler
is acycle<std::vector<int>>
numbers
is default-initializedcycler
is safely constructed afternumbers
Also, whenever you want to
using namespace std;
for convenience, only do it inside another (your own) namespace, never globally.
EDIT
I just noticed you're not using C++11. In this case, just use
typedef typename C::value_type& ref;
instead.
-
\$\begingroup\$ The parameter to getNext allows me to limit the cycling to the first 5 out of 10 elements for example. So if I want only the 10 elements all the time, then I won't need a parameter. \$\endgroup\$Dean– Dean2014年04月25日 08:59:00 +00:00Commented Apr 25, 2014 at 8:59
-
\$\begingroup\$ @kunkka_71 Ok, then maybe you could have two versions if you also need the automatic one. In this case, one overload could call the other. \$\endgroup\$iavr– iavr2014年04月25日 09:03:12 +00:00Commented Apr 25, 2014 at 9:03
-
\$\begingroup\$ after using typedef as you've mentioned, using ref = decltype(mContainer[0]); does not compile. decltype is C++11 only? \$\endgroup\$Dean– Dean2014年04月25日 11:09:21 +00:00Commented Apr 25, 2014 at 11:09
-
\$\begingroup\$ @kunkka_71 Yes, the last
typedef ...
line is a replacement forusing ref = ...
. \$\endgroup\$iavr– iavr2014年04月25日 13:17:22 +00:00Commented Apr 25, 2014 at 13:17
A few things about update()
:
It's less readable and maintainable to cram the functionality into one line. Just declare it within the class and define it outside. If the compiler decides that it should be inlined, it will do that for you.
Prefer
"\n"
overstd::endl
when just outputting a newline. The latter also flushes the buffer, which is slower and unneeded here. See this for more information.update
is not a very accurate name. What exactly does it update?
-
\$\begingroup\$ It's actually a stripped down code so the contents of update is not just that. And the couts are really just to output them on the screen. \$\endgroup\$Dean– Dean2014年04月25日 08:28:06 +00:00Commented Apr 25, 2014 at 8:28
A few remarks:
Try to avoid
using namespace std
.In
Processor
, you should also explicitly call the constructor of the vector.Your cycle class could be templated on the container type only, and use
TContainer::value_type
instead.