6
\$\begingroup\$

I've started reading C++ Primer a few weeks ago and there's an exercise that asks you to compare two arrays for equality. The code I made works, but is it good? What should I fix?

If the number of elements don't match, they're not equal. If the elements are different, they are not equal.

#include <iostream>

using namespace std;

bool arrayComparer(int *a, size_t asize, int *b, size_t bsize){
    if(asize != bsize){return false;}
    for(size_t ite = 0; ite < asize; ++ite){
       if(*(a+ite) != *(b+ite)){return false;}
    }
    return true;
}

int main()
{
    int ar1[]{0,1,2,3,4,5,6,7,8,9},
        *ptr1 = ar1,
        ar2[]{0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18},
        *ptr2 = ar2;

        size_t intSize = sizeof(int),
               asize = sizeof(ar1) / intSize,
               bsize = sizeof(ar2) / intSize;

    cout << arrayComparer(ptr1,asize,ptr2,bsize) << endl;
    return 0;
}

Since the number of elements may be different, I'm using pointers. I did not find a way to get the number of elements from the pointers, so I'm also passing the size to the function.

Comments are appreciated.

\$\endgroup\$
  • 1
    \$\begingroup\$ I'm assuming you're doing this for academic reasons, but just in case: there's no need for your loop. You can just do std::equal(ar1, ar1 + 10, ar2 + ar2 + 19). \$\endgroup\$ – Corbin Jul 11 '13 at 4:37
  • \$\begingroup\$ @Corbin, wouldn't ar1 == ar2 also work if they were both std::arrays (and of equal size)? \$\endgroup\$ – Jamal Jul 11 '13 at 5:19
  • 1
    \$\begingroup\$ @Jamal Yes. std::array's operator== does indeed do an element wise comparison. I would prefer std::equal though because it would allow him to later change either of the types involved without having to change the comparsion (If he used std::begin() instead of .begin()). (Also, std::array is C++11 which he might not be used.) \$\endgroup\$ – Corbin Jul 11 '13 at 6:13
  • \$\begingroup\$ @Corbin yes, I'm only doing it as an exercise. But thank you for pointing it out. \$\endgroup\$ – 2013Asker Jul 13 '13 at 1:15
5
\$\begingroup\$
  • Try not to use using namespace std.

  • You don't need an explicit return 0 at the end of main(). As reaching the end of main() already implies successful termination, the compiler will do the return for you.

  • In C++, use std::size_t over size_t, the former being part of the std namespace.

  • Consider having your function print true or false instead of 1 or 0. You could do this by putting std::boolalpha into the output stream before the function call.

    cout << std::boolalpha << arrayComparer(ptr1,asize,ptr2,bsize) << endl;
    
  • There is no need for this:

    std::size_t intSize = sizeof(int);
    

    You can just use sizeof(int) for both size calculations, so that you can only have size variables that correspond to array sizes. Extra possible overhead is not a problem because sizeof() is actually an operator, not a function.

  • If your compiler supports C++11, you can use an std::array instead of C-style arrays. If not, use an std::vector. They both know their own sizes, and you'll no longer need your own pointers.

    Raw pointer use in C++ should generally be avoided whenever possible, and passing raw pointers can introduce ownership issues. Passing C-style arrays to functions is also bad because they will decay to pointers. If you must use them anyway, at least for this exercise, then perhaps you can declare ptr1 and ptr2 within arrayComparer() instead of passing them from main().

| improve this answer | |
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.