Member Avatar for gretty

Hello

I have 2 functions that are just playing up. One is a function that takes in a string & should output the string backwards.
Problem: the syntax to insert a character into the string is wrong, I dont know the correct syntax.

The other is a function that takes in a string & converts each character to uppercase.
Problem: I dont know, as far as I can see, there is nothing wrong with the code, but it still outputs the string in a lowercase form, when it should be in uppercase

Any help would be greatly appreciated

Function 1:

string reverse(string s) {

	int size = (int)s.length();


	for (int i=0; i<size; i++) {
		s[i].insert( s[size-1] ); // I get an error here???
		s.erase(size);
	}
               return s;

Function 2:

string makeUpper(string s) {

	for (int i=0; i<(int)s.length(); i++) {
		if ( islower(s[i]) ) {
			toupper(s[i]);
		}
	}

	return s;
}

Code to call the functions:

#include <cstdlib>
#include <iostream>
#include <string>

using namespace std;

int countUpper(string s); // Task 1
string reverse(string s); // Task 2
string makeUpper(string s); // Task 3

int main() {

	string b = "abcde";
	string c = "abcde";

	cout << "String b reversed = " << reverse(b) << endl;
	cout << "String b capitalised = " << makeUpper(c) << endl;

	return 0;
}
  • 5 Contributors
  • 4 Replies
  • 214 Views
  • 14 Hours Discussion Span
  • Latest Post Latest Post by mrnutty

Dani AI

Generated

Common causes and quick fixes (answers consolidated from the replies)

  • The reverse attempt fails because s[i] is a char (not a string) and you can’t call string member functions on it. The insert/erase loop is fragile and off-by-one: s.erase(size) does not remove the last character the way the code expects. and were right to point toward in-place algorithms, and ’s idea of inserting characters into s is more complicated than necessary here.

  • The uppercase routine fails because std::toupper returns a value — it does not mutate the character in-place — and also expects either EOF or an unsigned char cast promoted to int. Passing a plain (possibly signed) char into std::toupper is undefined on some implementations.

Compact, safe replacements (prefer these over manual insert/erase loops)

// create a reversed copy in one line (uses reverse iterators)
std::string reversed(s.rbegin(), s.rend());
// uppercase in-place, avoiding undefined behaviour from signed chars
#include <algorithm>
#include <cctype>

std::transform(s.begin(), s.end(), s.begin(),
    [](unsigned char ch) -> char { return static_cast<char>(std::toupper(ch)); });

Practical notes and troubleshooting

  • For an in-place reverse use the standard algorithm std::reverse(s.begin(), s.end()) (very efficient). ’s pointer to the standard algorithm is sensible.
  • If the goal is to mutate the caller’s string, pass string&; if returning a modified copy is fine, pass by value and return the new string — modern compilers optimize copies well.
  • For non-ASCII text (UTF-8), std::toupper and byte-wise reversing will not work correctly; use a Unicode-aware library (ICU, or operate on codepoints) instead.

Credit: useful ideas seen from , , , and ; the code above avoids the pitfalls mentioned and follows safe std-library idioms.

Recommended Answers

All 4 Replies

Member Avatar for Ancient Dragon

First problem:

int main()
{
    string h = "abcde";
    std::reverse(h.begin(), h.end());

The problem with the second function is that the parameter is passed by value instead of by reference.

Member Avatar for Protuberance

The first error:
s - is not a string, is a char
Second:
inserting must be into begin, not into the end, so code

s[i].insert( s[size-1] );

is fully incorrect
Third:
Function insert has many overloads, but it's no string insert(char);

Correct code

string reverse(string s)
{
	int size = (int)s.length();

	char temp[2];
	for(int i = 0; i < size; i++)
	{
		temp[0] = s[size-1]; //keep symbol from the end
		temp[1] = '\0';
		s.insert(i, string(temp)); //inserting into begin
		s.erase(size); //erase last symbol
	}
	return s;
}
Member Avatar for macobex

Function 2:

the 5th line should be:
s = toupper(s);

toupper() fucntion is returning a value, it is not directly manipulating the string, so you still need to assign the current character to the returned value.

Hope it helps.

Member Avatar for mrnutty

This might be of help. Much easier way of reversing a string.

//note using mathematical notation its  [strt,end] and not [strt,end).
//strt is included and end is included as well.

void reverseRecursivly(string& str,int strt, int end)
{
	if(strt >= end || !str[0]) return;
	
	std::swap(str[strt],str[end]);
	reverseRecursivly(str,strt+1,end-1);
}

And also your makeUpper function is wrong :

string makeUpper(string s) {

	for (int i=0; i<(int)s.length(); i++) {
		if ( islower(s[i]) ) {
			s[i] = toupper(s[i]); //Revised
		}
	}

	return s;
}
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.