3
\$\begingroup\$

I want to know if there is a better way to square numbers in an array. This is what I wrote:

#include <iostream>
using std::cout;
using std::endl;


void square(int []);


int main()
{
    int array[] = {2,4,6,8,10};

    square(array);
    for(auto i : array)
        cout << i << " ";

}



void square(int array[])
{
    int size = sizeof(array)/sizeof(array[0]);
    for(int i=0; i< size; i++)
        array[i] *= array[i];
}
\$\endgroup\$
  • \$\begingroup\$ Note that in the body of square the number of elements in array are not known. sizeof(array) will be equivalent to sizeof(int*). This make the function producing wrong values. \$\endgroup\$ – Markus Mayer Oct 14 '14 at 7:30
  • \$\begingroup\$ Note: This is broken: int size = sizeof(array)/sizeof(array[0]); If you looked at your compiler warnings it will tell you this. When an array is passed as a parameter it decays into a pointer. So at the other end sizeof(array) will return you the size of a pointer. \$\endgroup\$ – Martin York Oct 14 '14 at 16:26
5
\$\begingroup\$
  1. int size = sizeof(array)/sizeof(array[0]);

    Just so you know, sizeof() returns size_t.

  2. You can use a vector instead of an array.

    vector<int> data = {2,4,6,8,10};
    
  3. Simplify squaring using std::transform() and use a lambda expression in place of a function object:

#include <iostream>
#include <algorithm>
#include <functional>

using std::cout;
using std::transform;
using std::vector;

int main()
{

    vector<int> data = {2,4,6,8,10};

    transform(data.begin(), data.end(), data.begin(), [](int x){return x*x;});


    for(auto i : data)
        cout << i << " ";

}
\$\endgroup\$
  • 2
    \$\begingroup\$ What about std::copy for the output? ;-) \$\endgroup\$ – Jamal Oct 14 '14 at 1:26
  • 1
    \$\begingroup\$ std::array<int, 5> is an alternative to std::vector. It does not use dynamic memory allocation. But it is not as flexible as vector. \$\endgroup\$ – Markus Mayer Oct 14 '14 at 7:30
  • \$\begingroup\$ @MarkusMayer: Not quite but close; there is a limit on array size enforced by the stack frame see so good for small and medium sized arrays. But it also forces you to specify the number of elements (5) before listing them. This makes maintenance slightly harder because if you need to expand the array you need to modify two locations (not just add the number(s) but set the new size). Not a huge disadvantage but it does violate the only touch one thing principle. \$\endgroup\$ – Martin York Oct 14 '14 at 16:30
  • \$\begingroup\$ @MarkusMayer: My philosophy is use std::vector as the defaul; when you refactor the code (because you now know what it is doing), then look for potential conversions from std::vector to std::array. \$\endgroup\$ – Martin York Oct 14 '14 at 16:34
3
\$\begingroup\$

(This looks like a homework question.)

void square(int array[])
{
    int size = sizeof(array)/sizeof(array[0]);

Here you have a major bug, which you would have noticed if you had compiled and run your code before posting it. Because array is a function parameter of type int*, sizeof(array) is the same as sizeof(int*). Therefore you square only the first 1 or 2 elements of your input array.

The most common idioms for passing around arrays in C++ are

void square(int *array, size_t length);  // a.k.a. "the C way"
void square(std::vector<int>& array);    // a.k.a. "dodging the question"
void square(int *begin, int *end);       // the STL does this, but template-ized
void square(int (&array)[5]);            // Pass an array by reference (not very useful as you need to specify the size) 
template<int N>
void square(int (&array)[N]);            // Use a template to make passing an
                                         // array by reference useful.
                                         // Note: You now no longer need to calculate the size (its N)

I guess my favorite "too advanced" answer for this one would be

#include <iostream>

template<class Array>
void square(Array& array) {
    for (auto& elt : array) {
        elt *= elt;
    }
}

int main() {
    int array[] = {2,4,6,8,10};
    square(array);
    for (auto i : array) {
        std::cout << i << " ";
    }
    std::cout << std::endl;
}
\$\endgroup\$
  • \$\begingroup\$ My most common idiom for passing an array is template<size_t size> void square(int (&array)[size]);. Did you forget that or am I weird? \$\endgroup\$ – nwp Oct 14 '14 at 14:26
  • \$\begingroup\$ @nwp: Then you are writing code that is not very friendly to use with the standard algorithms. It's probably best to start to look at modelling your code like the standard as it will fit better for re-use. \$\endgroup\$ – Martin York Oct 14 '14 at 16:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy