patterncppMinor
Multiply vector elements by a scalar value using STL and templates
Viewed 0 times
stlelementsscalarmultiplyvaluetemplatesusingandvector
Problem
I wrote a small scientific simulation where I wanted to manipulate vectors with the same kind of functionality that Python's NumPy vectors use. Namely, I wanted to be able to add vectors together, multiply by constants, etc. I have need for both vectors of type int and floating point, so I wanted to template the overloading. As an example (and for something to review), I overloaded scalar multiplication as:
The STL algorithm way I hinted at looks like this:
I would appreciate all criticism relevant to code, style, flow. My project is in C++11, so in theory I could use the newer range-based for loops for readability (I do in other parts of my code). Additionally, I think I could rewrite this using
template
vector operator*(const Q c, const vector &A) {
vector R(A.size());
for(int i=0;i<A.size();++i)
R[i] = c*A[i];
return R;
}The STL algorithm way I hinted at looks like this:
template
vector operator*(const Q c, const vector &A) {
vector R(A.size());
std::transform(A.begin(), A.end(), R.begin(),
std::bind1st(std::multiplies(),c));
return R;
}I would appreciate all criticism relevant to code, style, flow. My project is in C++11, so in theory I could use the newer range-based for loops for readability (I do in other parts of my code). Additionally, I think I could rewrite this using
std::transform, but I'm not sure if it would help improve the efficiency or the readability of my code.Solution
I would use the second version that you posted. Here are some problems with the first version:
This is a problem in both versions of your code. You pass
Update: Imagine somebody (maybe you) using your function.
For simplicity, let's say variable
When you do
Now in C++11, there is something called move semantics. This means, that without copy elision,
With copy elision, instead of creating
Here's how I would re-write the function:
You could do something like this:
C++11 has expanded the functionality of the
Update: The reason you want to use compatible types is because overflowing a signed number is undefined behavior in C++. This means anything can happen. Consider this example:
Different compilers on different platforms will do different things with the code above because there is undefined behavior (an
Programs typically use
Unless
This is more a subjective style guideline than anything.
- Prefer passing by value rather than making an explicit copy.
This is a problem in both versions of your code. You pass
A via constant reference. Normally this is a good thing, but then you then make a copy of A explicitly. In this case, it is possible to miss out on a compiler optimization called copy elision (second reference here). Passing by value would not cause any variable aliasing problems.Update: Imagine somebody (maybe you) using your function.
std::vector v1 = // ... insert elements
auto v2 = 3 * v1 ;For simplicity, let's say variable
v2 is located at address 0x10.When you do
3 * v1, you are calling your function. R would then be created at let's say address 0x20. When your function returns, the contents of R would have to be copied from 0x20 to v2 at 0x10 and then the destructor for R would have to be called. Without copy elision, this would require copying every element from R into v2. Now in C++11, there is something called move semantics. This means, that without copy elision,
R would not have to copy every element into v2, it could swap pointers. This is much faster, but it is still slower than copy elision. With copy elision, instead of creating
R at 0x20, it uses address 0x10 and builds v2 in place. With copy elision, nothing has to be copied and no pointers have to be swapped because v2 is built in place.Here's how I would re-write the function:
template
std::vector operator* (const Q c, std::vector A)
{
std::transform (A.begin (), A.end (), A.begin (),
std::bind1st (std::multiplies () , c)) ;
return A ;
}- If you have to use an index-loop for some reason, rather than a range-loop or iterators, make sure you are comparing compatible types.
You could do something like this:
typedef std::vector ::size_type size_type ;
for (size_type i = 0; i < A.size (); ++i)C++11 has expanded the functionality of the
using keyword. So instead of a typedef, you could also do:using size_type = decltype (A.size ()) ;Update: The reason you want to use compatible types is because overflowing a signed number is undefined behavior in C++. This means anything can happen. Consider this example:
const unsigned size = std::numeric_limits ::max () + 5 ;
std::vector v1 (size) ;
for (int n = std::numeric_limits ::max (); n < v1.size (); ++n) {
std::cout << n << ',' << v1.size () << '\n' ;
}Different compilers on different platforms will do different things with the code above because there is undefined behavior (an
int can never reach std::numeric_limits ::max () + 5).- Consider wrapping the
std::vectorinto a class.
Programs typically use
std::vector for many things. Overloading a global operator like you do would make me uncomfortable using your code.- Obligatory avoid
using namespace std;.
- Use more meaningful variable names and typenames.
Unless
A, R, Q, etc are commonly understood names for special variables in your scientific community, use more meaningful names.- Do not omit braces for if-statements or loops.
This is more a subjective style guideline than anything.
Code Snippets
std::vector <int> v1 = // ... insert elements
auto v2 = 3 * v1 ;template <class T, class Q>
std::vector <T> operator* (const Q c, std::vector <T> A)
{
std::transform (A.begin (), A.end (), A.begin (),
std::bind1st (std::multiplies <T> () , c)) ;
return A ;
}typedef std::vector <T>::size_type size_type ;
for (size_type i = 0; i < A.size (); ++i)using size_type = decltype (A.size ()) ;const unsigned size = std::numeric_limits <int>::max () + 5 ;
std::vector <char> v1 (size) ;
for (int n = std::numeric_limits <int>::max (); n < v1.size (); ++n) {
std::cout << n << ',' << v1.size () << '\n' ;
}Context
StackExchange Code Review Q#77546, answer score: 5
Revisions (0)
No revisions yet.