my sick unique

closed account (SECMoG1T)
Hi ... am getting a weird error in my function here and I just can't get around it, I will appreciate your help.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 bool myuniq(const string& s)
   {
       string temp=s;
       
       sort(temp.begin(),temp.end());
       auto uniq=unique(temp.begin(),temp.end());//error here
 
       if(uniq==temp.end())
        {
          return true;
        }

       else
         return false;
     }




error!  assignment of read-only location


Last edited on
There is nothing wrong with your function (except for the missing return statement).
http://coliru.stacked-crooked.com/a/9d2d0b6788aefa80

Favour passing by value over passing by reference to const and then making a copy.
And ideally, avoid using namespace std ; at global unnamed namespace scope.

1
2
3
4
5
6
7
8
#include <string>
#include <algorithm>

bool myuniq( std::string s )
{
    std::sort( s.begin(), s.end() );
    return std::unique( s.begin(), s.end() ) == s.end() ;
}
Last edited on
closed account (SECMoG1T)
Favour passing by value over passing by reference to const and then making a copy.

Thank you very much for that @JLBorges , I couldn't have reasoned that way hehe, your code looks smarter even but my problem up there is still giving me hell, note that my code above there have been trimmed to make it easy to read , actually I dint just want to return a bool if the string is unique but I also wanted to do some more evaluations using the unique iterator , hell is I can't assign the results of std::unique to anything due to some unheard of read-only error and am not willing to fall back and use a loop, well is there away maybe I can get this working, i would appreciate. Thanks
> my code above there have been trimmed to make it easy to read

Post the actual code that is generating the error.
(Or, if it is very long, a trimmed version that does not trim out the part that would generate the very same error.)
closed account (SECMoG1T)
Here it is..
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
#include <algorithm>
#include <iostream>
#include <string>

using namespace std;

bool isunique(const string&);
bool onlyspaces(const string&);

int main()
{
   string str;
   cout<<"enter a string";
   cin>>str;

   if(isunique(str))
    cout<<"string is  unique";
   else
    cout<<"string not unique";
}

bool isunique(const string& s)
{
    string temp=s;

    sort(temp.begin(),temp.end());
     auto myiter=unique(temp.begin(),temp.end());

    if(myiter==s.end())
        return true;
    else
    {
      return onlyspaces( string(myiter,temp.end()) );
    }
}

bool onlyspaces(const string& s)
{
    for(auto i: s)
        if(!isspace(i))
            return false;

        else
            return true;
}


Thank you.
Last edited on
Well, the code compiles cleanly.

Assuming that std::unique() works the way you think it does; there are still two logical errors.

1
2
// if(myiter==s.end()) // line 29
if( myiter == temp.end() )


and the else in line 43 should not be there.

The code, even with those corrections won't work as expected because after the call to std::unique():
Iterators pointing to an element between the new logical end and the physical end of the range are still dereferenceable, but the elements themselves have unspecified values.
http://en.cppreference.com/w/cpp/algorithm/unique


Something like this, perhaps:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
#include <string>
#include <algorithm>
#include <cctype>

bool myuniq( std::string s )
{
    // move all non-space characters to the beginning
    // http://en.cppreference.com/w/cpp/algorithm/remove
    const auto not_space_end = std::remove_if( s.begin(), s.end(), []( char c ) { return std::isspace(c) ; } ) ;

    std::sort( s.begin(), not_space_end ); // sort non-space characters

    return std::unique( s.begin(), not_space_end ) == not_space_end ; // are they unique?
}
closed account (SECMoG1T)
Now I get it
element between the new logical end and the physical end of the range are still dereferenceable I hadn't read that carefully , so my function onlyspaces shouldn't even exist woow, am so greatful , perhaps I will need to take some more time reading about algorithms ... they're very handy but you might mess things up, if you dont use them the right way.
Thank you very much for the help XD.
Topic archived. No new replies allowed.