Logic error with std::replace

Question

today I've encountered a logic error while working on a school project. For this project, switching chars around is required.

Say for example, the user enters the letter A. it is switched to U, and all is well. However, the issue occurs when the user enters the letter U. The character remains untouched. I've implemented two rules to switch from A to U and U to A. The string passes through both of the rules, and therefore is unchanged. How can I prevent this?

#include <iostream>
#include <algorithm>
#include <string>

using namespace std;

int main()
{
 string seq;
 getline(cin, seq);

 transform ( seq.begin(), seq.end(), seq.begin(), ::toupper );

 replace ( seq.begin(), seq.end(), 'A', 'U' );
 replace ( seq.begin(), seq.end(), 'U', 'A' );

 cout << seq;
 return 0;
}

The string is not unchanged. Consider a string like "U" demo — AndyG
– AndyG, Commented Apr 10, 2018 at 14:30
neither efficient nor nice: you could replace first A with a character x that otherwise does not appear in the string then U with a second charater y and then replace x with U and y with A respectively — 463035818_is_not_an_ai
– 463035818_is_not_an_ai, Commented Apr 10, 2018 at 14:32
Are you asking how to transform all 'A' to 'U' and all 'U' to 'A' without undoing any previous transformation? I think the answer lies in your usage of transform. Think about how you can modify this. — AndyG
– AndyG, Commented Apr 10, 2018 at 14:38

Caleth · Accepted Answer · 2018-04-10 14:45:43Z

10

With your implementation, you replace A to U then U back to A:

replace ( seq.begin(), seq.end(), 'A', 'U' );
replace ( seq.begin(), seq.end(), 'U', 'A' );

You need to make your replacements in one pass, for instance with std::transform:

std::transform(begin(seq), end(seq), begin(seq), some_function_object);

You just have to write this mysterious some_function_object. It can be a free-function:

char some_function_object(char input) { /* ... */ }
std::transform(begin(seq), end(seq), begin(seq), some_function_object);

or a lambda function:

std::transform(begin(seq), end(seq), begin(seq), [](char input) { /* ... */ } );

This function object must take a char and return:

A if input is U;
U if input is A;
input otherwise.

And how should you call it if its a free function? Well, why not a self-descriptive name like swapAandU?

edited Apr 10, 2018 at 14:45

Caleth

66k2 gold badges53 silver badges101 bronze badges

answered Apr 10, 2018 at 14:39

YSC

40.4k11 gold badges103 silver badges160 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Vlad from Moscow · Accepted Answer · 2018-04-11 13:37:48Z

2

It seems it is the case when a range-based for loop is better suitable and more efficient than using a standard algorithm as for example std::transform.

For example

#include <iostream>
#include <string>

int main() 
{
    std::string s( "AUAUA" );

    std::cout << s << std::endl;

    for ( char &c : s )
    {
        if ( c == 'A' ) c = 'U';
        else if ( c == 'U' ) c = 'A';
    }

    std::cout << s << std::endl;

    return 0;
}

The program output is

AUAUA
UAUAU

You can write a separate function as for example

#include <iostream>
#include <string>

std::string & convert( std::string &s1, const std::string &s2, const std::string &s3 )
{
    for ( char &c : s1 )
    {
        auto i = s2.find( c );
        if ( i != std::string::npos ) c = s3[i]; 
    }

    return s1;
}

int main() 
{
    std::string s( "AUAUA" );

    std::cout << s << std::endl;

    std::cout << convert( s, "AU", "UA" ) << std::endl;

    return 0;
}

If the string s2 is sorted then you can use the binary search algorithm instead of the linear search.

If you want to use a standard algorithm then in my opinion the most suitable algorithm is std::for_each that correcponds to the range-based for loop. For example the first demonstrative program can be rewritten using the algorithm the following way

#include <iostream>
#include <string>
#include <algorithm>

int main() 
{
    std::string s( "AUAUA" );

    std::cout << s << std::endl;

    std::for_each( s.begin(), s.end(), 
        []( char &c ) 
        { 
            if ( c == 'A' ) c = 'U';
            else if ( c == 'U' ) c = 'A';
        } );

    std::cout << s << std::endl;

    return 0;
}

edited Apr 11, 2018 at 13:37

answered Apr 10, 2018 at 14:44

Vlad from Moscow

313k27 gold badges204 silver badges358 bronze badges

3 Comments

Koi Over a year ago

This code worked the way I wanted it to. Thank you very much!

YSC Over a year ago

"more efficient than using a standard algorithm as for example std::transform" [citation needed] ;)

Vlad from Moscow Over a year ago

@YSC Redundant assignments are absent.

Collectives™ on Stack Overflow

Logic error with std::replace

2 Answers 2

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related