Range-based for loop ambiguity in C++

Question

These two pieces of code:

A)

void KMeans<N>::assign_pixels_to_clusters()
{
    for (auto cluster : clusters)
    {
        cluster.clear_pixels();
    }

    for (auto pixel : pixels)
    {
        float min_distance = FLT_MAX;
        Cluster<N> c;

        for (auto cluster : clusters)
        {
            float distance = norm(
                pixel.get_value(),
                cluster.get_centroid()
            );

            if (distance < min_distance)
            {
                min_distance = distance;
                c = cluster;
            }
        }

        c.add_pixel(pixel);
    }
}

and B)

template <size_t N>
void KMeans<N>::assign_pixels_to_clusters()
{
    for (auto cluster : clusters)
    {
        cluster.clear_pixels();
    }

    for (auto pixel : pixels)
    {
        float min_distance = FLT_MAX;
        int idx = 0;

        for (int i = 0; i < no_of_clusters; i++)
        {
            float distance = norm(
                pixel.get_value(),
                clusters[i].get_centroid()
            );

            if (distance < min_distance)
            {
                min_distance = distance;
                idx = i;
            }
        }

        clusters[idx].add_pixel(pixel);
    }
}

seem similar to me, yet only B) works the way I want to. In the case of A), the pixels are not assigned to clusters at all. After the piece of code A) is run, the clusters are empty and have no pixels assigned to them. Can you help me understand why, please?

I do not understand. Yes, `Cluster c;` is a different separate object then `clusters[idx]`. — KamilCuk, Apr 24 '20 at 22:16
Your range-for loops are odd. It seems you want `for(auto &cluster:clusters)`. The first loop does nothing as written. — cigien, Apr 24 '20 at 22:18
@KamilCuk ```clusters``` is a vector of objects of type ```Cluster``` — Andrei, Apr 24 '20 at 22:19
@cigien can you please explain to me why is the ```&``` operator needed? I am completely new to the object-oriented side of C++ and some things are a bit unclear to me. — Andrei, Apr 24 '20 at 22:22
I've answered this particular question, but I would suggest picking a nice [book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and learning concepts from there. — cigien, Apr 24 '20 at 22:30

cigien · Accepted Answer · 2020-04-24T22:31:07.100

In your A version, you are making a copy of the cluster on this line:

c = cluster;

So when you do

c.add_pixel(pixel);

you are not changing the clusters range.

For the same reason, this loop:

for (auto cluster : clusters)
    {
        cluster.clear_pixels();
    }

doesn't actually clear pixels of any cluster in clusters, since each cluster is a copy. If you actually want to refer to each cluster in clusters, you need to do this:

for (auto & cluster : clusters)
    {
        cluster.clear_pixels();
    }

Range-based for loop ambiguity in C++

1 Answers1