0

Consider:

#include <stdio.h>
#include <set>

void printset(std::set<int>& Set) {
    for (std::set<int>::iterator siter = Set.begin(); siter != Set.end(); ++siter) {
        int val = *siter;
        printf("%d ", val);
    }
}

void printsetofset0(std::set<std::set<int>>& SetofSet) {
    for (std::set<std::set<int>>::iterator siter = SetofSet.begin(); siter != SetofSet.end(); ++siter) {
        std::set<int> Set = *siter;
        printset(Set);
    }
}

void printsetofset1(std::set<std::set<int>>& SetofSet) {
    for (std::set<std::set<int>>::iterator siter = SetofSet.begin(); siter != SetofSet.end(); ++siter) {
        printset(*siter);//this line gives error
    }
}   

printsetofset1 with the printset(*siter); gives error:

<source>: In function 'void printsetofset1(std::set<std::set<int> >&)':
<source>:20:34: error: binding reference of type 'std::set<int>&' to 'const std::set<int>' discards qualifiers
   20 |                         printset(*siter);
      |                                  ^~~~~~
<source>:4:38: note:   initializing argument 1 of 'void printset(std::set<int>&)'
    4 |         void printset(std::set<int>& Set) {
      |                       ~~~~~~~~~~~~~~~^~~
Compiler returned: 1

See Godbolt Link here.

printsetofset0 with the lines: std::set<int> Set = *siter; printset(Set);

compiles and works just fine.

What is the reason why one printsetofset0 works, while seemingly functionally equivalent (and shorter) preintsetofset1 does not work?

Tryer
  • 3,580
  • 1
  • 26
  • 49
  • 1
    The elements, which the iterator points to, are constant. You can read it here in the [Notes](https://en.cppreference.com/w/cpp/container/set/begin): "Because both iterator and const_iterator are constant iterators (and may in fact be the same type), it is not possible to mutate the elements of the container through an iterator returned by any of these member functions.". The idea is: changing the elements would require to reorder the set (internally). You can solve the situation by adding a `const` modifier to the parameter of your `printset` function. – J.P.S. Mar 20 '22 at 08:36

3 Answers3

3

The elements, which the iterator points to, are constant. You can read it here in the Notes: Because both iterator and const_iterator are constant iterators (and may in fact be the same type), it is not possible to mutate the elements of the container through an iterator returned by any of these member functions.. But, passing a reference to your printset function would violate this property.

Internally, sets allow fast access via index structures. Changing the elements would require to reorder the set (internally).

You can solve the situation by adding a const modifier to the parameter of your printset function:

void printset(const std::set<int>& Set)
J.P.S.
  • 485
  • 4
  • 11
  • So, in `printsetofset0`, when I pass `Set` which was `Set=*siter`, it automatically gets restricted to a `const` when passed as an argument even though `printset` as of now is setup to accept only a plain `iterator` and not a `const_iterator`. Would not any change to `Set` also implicitly change `*siter` since I am passing it by reference ? – Tryer Mar 20 '22 at 08:45
  • 1
    No, in `printofset0`, you make a copy of it. That's why you don't violate the *immutable* property of the outer set. – J.P.S. Mar 20 '22 at 08:47
  • After thinking about this a bit more: You are allowed to modify a set since passing a set by reference is allowed. You are *not* allowed to modify a set via its iterator by saying something like `(*siter).insert(4);` Is this correct? – Tryer Mar 20 '22 at 09:19
  • 1
    Right, you could not change the content of the iterator, since the iterator is const. It could also change the order of the surrounding set. – J.P.S. Mar 20 '22 at 09:27
0

All your print functions should take their argument by const reference, because they don't need to modify the argument. Like this:

void printset(const std::set<int>& Set)

I also suggest a range-based for loop to simplify your code:

for (int val : Set)

The reason printset cannot take a non-const reference is that you're passing it the set-within-a-set, and the values stored in sets are always const. For more on this, see: Why does std::set seem to force the use of a const_iterator?

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • I agree that it is good practice, however, I do not think that is the reason for the compilation error. – Tryer Mar 20 '22 at 08:33
  • Fair enough! I still don't quite know why your suggestion works. As I see it, both `printsetofset0` and `printsetofset1` are functionally equivalent. Obviously, I am wrong there! – Tryer Mar 20 '22 at 08:39
  • @Tryer The comment on your post explains it. Unfortunately, this answer doesn't. – Passer By Mar 20 '22 at 08:40
0

The problem is that although the set types define both the iterator and const_iterator types, both types of iterators give us read-only access to the elements in the set. That is, the keys in a set are const. We can use a set iterator to read, but not write, an element’s value.

So to solve the error in your program you need to add a low-level const to the parameter named Set of the printSet function as shown below:

//------------vvvvv--------------------------->low-level const added here
void printset(const std::set<int>& Set) {
        //other code as before
    }

Demo

Jason
  • 36,170
  • 5
  • 26
  • 60
  • Unrelated: please consider using bold face markup [only in exceptional cases](https://meta.stackoverflow.com/a/404404/6868543), it greatly [impairs readability](https://meta.stackoverflow.com/a/262086/6868543). – j6t Mar 20 '22 at 09:23
  • @j6t These(using boldface etc) options are so that we can easily shift the focus on the main parts of a text. Also, it is opinion based whether my use of boldface etc impairs readability or not. It helps me in reading texts so i tend to used them wherever i find appropriate. I use them in all my answers regularly. It is my choice to write my answer in whichever way/style i see fit. If there is something technically wrong with my answer, then you can point that out. – Jason Mar 20 '22 at 09:49