While loop iterating on too many elements in C++

Question

When I run the code I get all those numbers as outputs which means that my while loop seems to be going over elements that it should not got over. Why is this happening ? For context I am currently learning C++ with a tour of C++ and I am very confused with pointer and references.

This is where I got the code: Buggy code in "A Tour of C++" or non-compliant compiler?

int counter(int* arr,int c){
    int counter = 0;
    while(*arr){
    cout << *arr<<"\n";
    if(*arr == c){
        ++counter;
    }
    ++arr;
    }
    return counter;
}

int main()
{
    int arr[3] = {1,2,3};
    int count = counter(arr,1);
    cout<< count;
}

Example run:

/Users/benediktschesch/CLionProjects/untitled/cmake-build-debug/untitled
1
2
3
-945684358
-1153026697
-280532248
32766
1839025881
32767
1839025881
32767
1
Process finished with exit code 0

Answer 1

This is very similar to not providing a null terminator for a character array that will be used as a string.

while(*arr) 

means stop when you find a zero.

int arr[3] = {1,2,3};

provides no zero, therefore you have no control over when the loop will stop.

TL;DR Solution:

Use a Library container. std::vector or std::array would be a good fit here, as would std::count from the <algorithm> library and std::begin and std::end from the <iterator> library.

#include <iostream>
#include <iterator>
#include <algorithm>

int main()
{
    int arr[] = { 1, 2, 3 };
    int count = std::count(std::begin(arr), std::end(arr), 1);
    std:: cout << count;
}

Explanation:

You could provide a zero

int arr[] = {1,2,3,0};

Note I remove the explicit array size. It's not needed because the compiler knows from the number of elements in the initialzer.

Note also that this will stop upon reaching the first zero, so

int arr[] = {1,2,3,0,1,2,3,0};

will only spot one 1. This makes zero a very poor value to use to terminate a list of integers unless 0 is guaranteed to not be in the input.

To scan the whole array and only the array, the size of the array needs to be provided. This can be done by passing in a size parameter

int counter(int* arr, size_t len, int c)
{
    int counter = 0;
    while (len--)
    {
        std::cout << *arr << "\n";
        if (*arr == c)
        {
            ++counter;
        }
        ++arr;
    }
    return counter;
}

int main()
{
    int arr[3] = { 1, 2, 3 };
    int count = counter(arr, std::size(arr), 1);
    std:: cout << count;
}

but the preferred solution in Modern C++ would be to use a container in place of the array. Containers know their size and offer up a wide variety of other tools to make writing code easier and less error-prone.

#include <iostream>
#include <vector>

int counter(const std::vector<int> & arr, int c)
{
    int counter = 0;
    for (const auto & val: arr)
    {
        std::cout << val << "\n";
        if (val == c)
        {
            ++counter;
        }
    }
    return counter;
}

int main()
{
    std::vector<int> arr = { 1, 2, 3 };
    int count = counter(arr, 1);
    std:: cout << count;
}

Note the use of a range-based for loop to simplify the code. const auto & val deduces the type of val from the contents of arr with auto. The value is not going to be changed as a result of the loop so we declare it const to prevent accidents and make it a reference because maybe the compiler can pull off some extra optimization voodoo. In addition you can keep reusing this exact statement without having to change a thing if the container or the type of data in the container is ever changed. This prevents mistakes later when maintaining the code.

You could also use std::array and make counter a templated function that detects the size of the std::array here, but that's a bit much at this point.

The next evolution takes advantage of the <algorithm> library.

#include <iostream>
#include <vector>
#include <algorithm>

int main()
{
    std::vector<int> arr = { 1, 2, 3 };
    int count = std::count(arr.begin(), arr.end(), 1);
    std:: cout << count;
}

In this case Iterators are being used rather than specifying lengths. This lets you easily scan subsets of a container.

This allows us to go back to using a vanilla array by taking advantage of std::begin and std::end to turn the array into a pair of iterators :

#include <iostream>
#include <iterator>
#include <algorithm>

int main()
{
    int arr[] = { 1, 2, 3 };
    int count = std::count(std::begin(arr), std::end(arr), 1);
    std:: cout << count;
}

And that brings us around to the TL;DR solution.

Answer 2

I recommend to use std::array and range based for loops

#include <array>
#include <iostream>

int counter(const std::array<int, 3> &arr, int c){
    int counter = 0;
    for (auto const a : arr) {
        std::cout << a << "\n";
        if(a == c){
            ++counter;
        }
    }
    return counter;
}

int main()
{
    std::array<int, 3> arr = {1,2,3};
    int count = counter(arr,1);
    std::cout << count;
}

The reason for your problem is that in the line

while(*arr){

the statement

*arr

is evaluated as boolean. It is false for *arr == 0 and true in any other case. In your code you need to get the size of the array or a last element with value 0. There are different ways. You can add a last element with 0 or you can pass the size to the function. But C++ standard provides stl containers that solve your problem without overhead. std::array is such a container that contains the data and the size of your array. It is a template class so that there is no extra data needed for the size. First you should learn how to use the tools that a language provides like stl containers and algorithms. Later you can learn how to use low level functions and data types.

Answer 3

Except character arrays that contain strings all other arrays (if you do not use intentionally a sentinel value) do not contain a zero element that signals the end of an array. So your function can invoke undefined behavior if the array does not contain an element equal to 0 (as in your case).

So in general your function should have one more parameter that specifies the number of elements in the array and should look like

size_t counter( const int *arr, size_t n, int value )
{
    size_t count = 0;

    for ( const int *p = arr; p != arr + n; ++p )
    {
        if ( *p == value ) ++count;
    }

    return count;
}

and called like

int main()
{
    int arr[] = { 1, 2, 3 };
    const size_t N = sizeof( arr ) / sizeof( *arr );

    size_t count = counter( arr, N, 1 );

    std::cout << count << '\n';
}

If your compiler supports C++ 17 then instead of the expression sizeof( arr ) / sizeof( *arr ) you can include standard function std::size() declared in the header <iterator> like std::size( arr ).

Otherwise instead of the expression sizeof( arr ) / sizeof( *arr ) you could use expression

std::extent<decltype( arr )>::value

provided that the header <type_traits> is included.

Take into account that there is standard algorithm std::count declared in header <algorithm> that performs the same task. Here is a demonstrative program

#include <iostream>
#include <iterator>
#include <algorithm>

int main()
{
    int arr[] = { 1, 2, 3 };

    auto count = std::count( std::begin( arr ), std::end( arr ), 1 );

    std::cout << count << '\n';
}