remove_copy_if

Just practicing algorithms. Can anybody point out why temp is null after using remove_copy_if

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
#include<iostream>
#include<exception>
#include<algorithm>
#include<string>
#include<iterator>
using namespace std;

char non_white(const char& ch) {
	if (ch == ' ') {
		return char(0);
	}else {
		return ch;
	}
}

char lowercase(char& ch) {
	if (int(ch) <= 90 && int(ch) >= 65) {
		return char(int(ch) + 32);
	}
	return ch;
}

bool is_palindrome(const string& phrase) {
	string temp;
	string::iterator it = temp.begin();
	
	remove_copy_if(phrase.begin(), phrase.end(), inserter(temp,it), non_white);
	cout << temp << endl;
	transform(temp.begin(), temp.end(), temp.begin(), lowercase);
	cout << temp << endl;
	return equal(temp.begin(), temp.end(), temp.rbegin());

}


int main() {
	cout << "Enter String :";
	string input;
	getline(cin, input);
	if (is_palindrome(input))
		cout << "Palindrome!!"<<endl;
	else
		cout << "Not Palindrome!"<<endl;
}
Are you sure that you didn't mean copy_if and not remove_copy_if ?
remove_copy_if copies the elements for which the function returns false.

The non_white function only returns false (zero) for spaces which means only the spaces will be copied to temp.
1
2
3
4
string temp;
string::iterator it = temp.begin();

remove_copy_if(phrase.begin(), phrase.end(), inserter(temp,it), non_white);

‘temp’ is not large enough to contain ‘phrase’, even without spaces:

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
47
48
49
50
51
52
53
54
55
56
#include <algorithm>
#include <cctype>
#include <iostream>
#include <iterator>
#include <string>

bool is_palindrome(const std::string& phrase);
bool non_white(const char& ch);
char lowercase(char& ch);
void waitForEnter();

int main()
{
    std::string input = "Lorem ipsum dolor sit amet, an vim viris animal "
                        "dolorum, in mei legere vocent";
    if (is_palindrome(input)) { std::cout << "Palindrome!!\n"; }
    else                      { std::cout << "Not Palindrome!\n"; }
    input = "In girum imus nocte et consumimur igni";
    if (is_palindrome(input)) { std::cout << "Palindrome!!\n"; }
    else                      { std::cout << "Not Palindrome!\n"; }
    waitForEnter();
    return 0;
}

bool is_palindrome(const std::string& phrase)
{
    std::string temp(phrase.length(), ' ');
    std::remove_copy_if(phrase.begin(), phrase.end(), temp.begin(), non_white);
    std::cout << "temp: \"" << temp << "\"\n";
    temp.erase(std::remove_if(temp.begin(), temp.end(), ::isspace), 
                temp.end());
    std::transform(temp.begin(), temp.end(), temp.begin(), lowercase);
    std::cout << "temp: \"" << temp << "\"\n";
    return equal(temp.begin(), temp.end(), temp.rbegin());

}

bool non_white(const char& ch)
{
    if (ch == ' ') { return true; }
    return false;
}

char lowercase(char& ch)
{
    if (int(ch) <= 90 && int(ch) >= 65) {
        return char(int(ch) + 32);
    }
    return ch;
}

void waitForEnter()
{
    std::cout << "\nPress ENTER to continue...\n";
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
}


Passing a string by reference just to make a copy of it inside the function doesn’t improve efficiency.
Hints for a simplified version of your code:
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
#include <algorithm>
#include <cctype>
#include <iostream>
#include <iterator>
#include <string>

bool is_palindrome(std::string phrase);
void waitForEnter();

int main()
{
    std::string input = "Lorem ipsum dolor sit amet, an vim viris animal "
                        "dolorum, in mei legere vocent";
    if (is_palindrome(input)) { std::cout << "Palindrome!!\n"; }
    else                      { std::cout << "Not Palindrome!\n"; }
    input = "In girum imus nocte et consumimur igni";
    if (is_palindrome(input)) { std::cout << "Palindrome!!\n"; }
    else                      { std::cout << "Not Palindrome!\n"; }
    waitForEnter();
    return 0;
}

bool is_palindrome(std::string phrase)
{
    std::cout << "phrase: \"" << phrase << "\"\n";
    phrase.erase(std::remove_if(phrase.begin(), phrase.end(), ::isspace), 
                phrase.end());
    std::transform(phrase.begin(), phrase.end(), phrase.begin(), ::tolower);
    std::cout << "phrase: \"" << phrase << "\"\n";
    return equal(phrase.begin(), phrase.end(), phrase.rbegin());
}

void waitForEnter()
{
    std::cout << "\nPress ENTER to continue...\n";
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
}

Enoizat wrote:
‘temp’ is not large enough to contain ‘phrase’, even without spaces

That's why he used std::inserter that constructs a std::insert_iterator which will automatically increase the size of the container as necessary.
Peter87 wrote:
That's why he used std::inserter

Too right!!
Thanks a lot for your correction, Peter87, I totally missed that point.
The following version follows closely keh k lenge’s original code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
bool is_palindrome(const std::string& phrase)
{
    std::string temp;
    std::remove_copy_if(phrase.begin(), phrase.end(), 
                        std::inserter(temp, temp.begin()), non_white);
    std::cout << "temp: \"" << temp << "\"\n";
    std::transform(temp.begin(), temp.end(), temp.begin(), lowercase);
    std::cout << "temp: \"" << temp << "\"\n";
    return equal(temp.begin(), temp.end(), temp.rbegin());
}

bool non_white(const char& ch)
{
    if (ch == ' ') { return true; }
    return false;
}

Topic archived. No new replies allowed.