473,327 Members | 2,065 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,327 software developers and data experts.

Problem with string

I did this function:

void clean(string&s)
{
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
s[i]=tolower(s[i]);
else
s.erase(i,1);
}
return;
}

I'm having a strange behavior with string like these:

"Disneyland" -> Disneyland
".disneyland." -> .disneyland"

It seems as if there is something going on with the erase function that
i don't know, because with this:

void clean(string&s)
{
string s2;
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
{
s[i]=tolower(s[i]);
s2+=s[i];
}
}
s=s2;
return;
}

Everything goes OK.

What's wrong with the first code?

Thanks.

Jun 3 '06 #1
11 1742
Gaijinco wrote:
I did this function:

void clean(string&s)
{
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
s[i]=tolower(s[i]);
else
s.erase(i,1);
}
return;
}
A call to erase() does not just rub off the content and left a hole in
the string. It moves the subsequent content forward to replace the part
to be removed.

As a result of moving the content, your index i no longer indexes into
the character that you want.

I'm having a strange behavior with string like these:

"Disneyland" -> Disneyland
".disneyland." -> .disneyland"

It seems as if there is something going on with the erase function that
i don't know, because with this:
....

void clean(string&s)
{
string s2;
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
{
s[i]=tolower(s[i]);
s2+=s[i];
}
}
s=s2;
return;
}

Everything goes OK.
Yep, this is the right way to do it. It is a cleaner, faster and, above
all, correct implementation.

What's wrong with the first code?
Index are much like iterators, they are "invalidated" when you erase
something from a container.

Thanks.


Ben
Jun 3 '06 #2
Gaijinco wrote:
I did this function:

void clean(string&s)
{
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
s[i]=tolower(s[i]);
else
s.erase(i,1);
}
return;
}

I'm having a strange behavior with string like these:

"Disneyland" -> Disneyland
".disneyland." -> .disneyland"

It seems as if there is something going on with the erase function that
i don't know, because with this:

void clean(string&s)
{
string s2;
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
{
s[i]=tolower(s[i]);
s2+=s[i];
}
}
s=s2;
return;
}

Everything goes OK.

What's wrong with the first code?

Thanks.


Well yes, erase() removes chars, so the size has changed.
Here's a quick and dirty example:

#include <iostream>
#include <string>
#include <cctype>

void clean( std::string& s )
{
std::string::iterator it;

// step thru the string using the iterator 'it'
for( it = s.begin(); it != s.end(); )
{
// if the current char is an alpha
if( isalpha(*it) )
{
// change the char to lower case
*it = tolower(*it);

// increment 'it' to the next char
++it;
}
else
{
// delete the current non-alpha char from 's'.
// resets 'it' to point to the first char after
// the one just deleted - or to s.end() if no
// more chars in 's'.
it = s.erase(it);
}
}

return;
}

int main()
{
std::string t1 = "Disneyland";
std::string t2 = ".disneyland.";

clean(t1);
clean(t2);

std::cout << t1 << std::endl;
std::cout << t2 << std::endl;

return 0;
}

This program outputs:

disneyland
disneyland

Regards,
Larry
Jun 3 '06 #3
dj
Larry I Smith wrote:
Gaijinco wrote:
I did this function:

void clean(string&s)
{
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
s[i]=tolower(s[i]);
else
s.erase(i,1);
}
return;
}

I'm having a strange behavior with string like these:

"Disneyland" -> Disneyland
".disneyland." -> .disneyland"

It seems as if there is something going on with the erase function that
i don't know, because with this:

void clean(string&s)
{
string s2;
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
{
s[i]=tolower(s[i]);
s2+=s[i];
}
}
s=s2;
return;
}

Everything goes OK.

What's wrong with the first code?

Thanks.


Well yes, erase() removes chars, so the size has changed.
Here's a quick and dirty example:

#include <iostream>
#include <string>
#include <cctype>

void clean( std::string& s )
{
std::string::iterator it;

// step thru the string using the iterator 'it'
for( it = s.begin(); it != s.end(); )
{
// if the current char is an alpha
if( isalpha(*it) )
{
// change the char to lower case
*it = tolower(*it);

// increment 'it' to the next char
++it;
}
else
{
// delete the current non-alpha char from 's'.
// resets 'it' to point to the first char after
// the one just deleted - or to s.end() if no
// more chars in 's'.
it = s.erase(it);
}
}

return;
}

int main()
{
std::string t1 = "Disneyland";
std::string t2 = ".disneyland.";

clean(t1);
clean(t2);

std::cout << t1 << std::endl;
std::cout << t2 << std::endl;

return 0;
}

This program outputs:

disneyland
disneyland

Regards,
Larry


In cases like this, i prefer to traverse the container in reverse order.
In that case the part of the container we have already examined is
shifted and presents no problem.

Wouldn't you agree?
Jun 3 '06 #4
Thank you for all your opinions! This was quite insightful!

Jun 3 '06 #5
In article <11**********************@u72g2000cwu.googlegroups .com>,
"Gaijinco" <ga******@gmail.com> wrote:
I did this function:

void clean(string&s)
{
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
s[i]=tolower(s[i]);
else
s.erase(i,1);
}
return;
}

I'm having a strange behavior with string like these:

"Disneyland" -> Disneyland
".disneyland." -> .disneyland"

It seems as if there is something going on with the erase function that
i don't know


Think about what happens when you erase an element of the container. All
the elements that come after are shifted one toward the front of the
container. So, for example, when the first quote is erased, the period
is moved to element 0, then 'i' is incremented to be equal to 1, so the
period is never checked.

There are a couple of ways to fix this bu why don't you take a stab at
it first.

--
Bene disserere est finis logices. -- Aristotle
Jun 3 '06 #6
In article <dv********************@news.siol.net>,
dj <sm*******@lycos.com> wrote:
In cases like this, i prefer to traverse the container in reverse order.
In that case the part of the container we have already examined is
shifted and presents no problem.

Wouldn't you agree?


No, still too many assignments and redundant checks when an element is
removed. Better would be:

void clean( string& s )
{
string::size_type insert = 0;
for ( string::size_type index = 0; index != s.size(); ++index )
if ( isalpha( s[index] ) )
s[insert++] = tolower( s[index] );
s.erase( insert, string::npos );
}

That way, each character is only moved once. (strictly speaking that
last line could be "s.erase( insert );" but that doesn't make it clear
that all elements from insert to the end of the string are being erased.)

--
Bene disserere est finis logices. -- Aristotle
Jun 3 '06 #7
In article <44***********************@news.optusnet.com.au> ,
benben <be******@yahoo.com.au> wrote:
void clean(string&s)
{
string s2;
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
{
s[i]=tolower(s[i]);
s2+=s[i];
}
}
s=s2;
return;
}

Everything goes OK.


Yep, this is the right way to do it. It is a cleaner, faster and, above
all, correct implementation.


It isn't faster for two reasons. (1) s2 needs to call new[], possibly
multiple times and each time it must copy the contents of the old array
into the new array (2) after s2 is filled in, it is assigned back into
's' (yet another loop.)

See my previous post for a more appropriate implementation (which is
based on the remove_copy_if algorithm.)

--
Bene disserere est finis logices. -- Aristotle
Jun 3 '06 #8
dj wrote:
Larry I Smith wrote:
Gaijinco wrote:
I did this function:

void clean(string&s)
{
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
s[i]=tolower(s[i]);
else
s.erase(i,1);
}
return;
}

I'm having a strange behavior with string like these:

"Disneyland" -> Disneyland
".disneyland." -> .disneyland"

It seems as if there is something going on with the erase function that
i don't know, because with this:

void clean(string&s)
{
string s2;
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
{
s[i]=tolower(s[i]);
s2+=s[i];
}
}
s=s2;
return;
}
Everything goes OK.

What's wrong with the first code?

Thanks.


Well yes, erase() removes chars, so the size has changed.
Here's a quick and dirty example:

#include <iostream>
#include <string>
#include <cctype>

void clean( std::string& s )
{
std::string::iterator it;

// step thru the string using the iterator 'it'
for( it = s.begin(); it != s.end(); )
{
// if the current char is an alpha
if( isalpha(*it) )
{
// change the char to lower case
*it = tolower(*it);

// increment 'it' to the next char
++it;
}
else
{
// delete the current non-alpha char from 's'.
// resets 'it' to point to the first char after
// the one just deleted - or to s.end() if no
// more chars in 's'.
it = s.erase(it);
}
}

return;
}

int main()
{
std::string t1 = "Disneyland";
std::string t2 = ".disneyland.";

clean(t1);
clean(t2);

std::cout << t1 << std::endl;
std::cout << t2 << std::endl;

return 0;
}

This program outputs:

disneyland
disneyland

Regards,
Larry


In cases like this, i prefer to traverse the container in reverse order.
In that case the part of the container we have already examined is
shifted and presents no problem.

Wouldn't you agree?


I didn't intend to optimize the original 'clean()', just make
it work correctly; otherwise, I might have been tempted to use
'remove_if()' and/or 'translate()'

Regards,
Larry
Jun 3 '06 #9
dj
Larry I Smith wrote:
dj wrote:
Larry I Smith wrote:
Gaijinco wrote:
I did this function:

void clean(string&s)
{
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
s[i]=tolower(s[i]);
else
s.erase(i,1);
}
return;
}

I'm having a strange behavior with string like these:

"Disneyland" -> Disneyland
".disneyland." -> .disneyland"

It seems as if there is something going on with the erase function that
i don't know, because with this:

void clean(string&s)
{
string s2;
for(size_t i=0; i<s.size(); ++i)
{
if(isalpha(s[i]))
{
s[i]=tolower(s[i]);
s2+=s[i];
}
}
s=s2;
return;
}
Everything goes OK.

What's wrong with the first code?

Thanks.

Well yes, erase() removes chars, so the size has changed.
Here's a quick and dirty example:

#include <iostream>
#include <string>
#include <cctype>

void clean( std::string& s )
{
std::string::iterator it;

// step thru the string using the iterator 'it'
for( it = s.begin(); it != s.end(); )
{
// if the current char is an alpha
if( isalpha(*it) )
{
// change the char to lower case
*it = tolower(*it);

// increment 'it' to the next char
++it;
}
else
{
// delete the current non-alpha char from 's'.
// resets 'it' to point to the first char after
// the one just deleted - or to s.end() if no
// more chars in 's'.
it = s.erase(it);
}
}

return;
}

int main()
{
std::string t1 = "Disneyland";
std::string t2 = ".disneyland.";

clean(t1);
clean(t2);

std::cout << t1 << std::endl;
std::cout << t2 << std::endl;

return 0;
}

This program outputs:

disneyland
disneyland

Regards,
Larry

In cases like this, i prefer to traverse the container in reverse order.
In that case the part of the container we have already examined is
shifted and presents no problem.

Wouldn't you agree?


I didn't intend to optimize the original 'clean()', just make
it work correctly; otherwise, I might have been tempted to use
'remove_if()' and/or 'translate()'

Regards,
Larry


Going reverse is by no means optimization - it is exactly what you said
you try to achieve - correctness. The only difference is in the "for" line.
Jun 3 '06 #10
dj
Daniel T. wrote:
In article <dv********************@news.siol.net>,
dj <sm*******@lycos.com> wrote:
In cases like this, i prefer to traverse the container in reverse order.
In that case the part of the container we have already examined is
shifted and presents no problem.

Wouldn't you agree?


No, still too many assignments and redundant checks when an element is
removed. Better would be:

void clean( string& s )
{
string::size_type insert = 0;
for ( string::size_type index = 0; index != s.size(); ++index )
if ( isalpha( s[index] ) )
s[insert++] = tolower( s[index] );
s.erase( insert, string::npos );
}

That way, each character is only moved once. (strictly speaking that
last line could be "s.erase( insert );" but that doesn't make it clear
that all elements from insert to the end of the string are being erased.)


I don't believe I follow you - what redundant checks and assignments?

void clean( string& s )
{
for ( string::size_type index = s.size()-1; index >= 0; --index ) {
if ( isalpha( s[index] ) )
s[index] = tolower( s[index] );
else
s.erase( index, 1 );
}
}

Of course, the code with single erase is certainly faster when multiple
erasures are needed, though it includes an additional post-increment
(which itself expands into an assignment and increment). However, if the
container stores objects that need destruction (like classes), the code
with single erase is not useful (overwriting in-place would leave
dangling objects).
Jun 3 '06 #11
In article <w2********************@news.siol.net>,
dj <sm*******@lycos.com> wrote:
Daniel T. wrote:
In article <dv********************@news.siol.net>,
dj <sm*******@lycos.com> wrote:
In cases like this, i prefer to traverse the container in reverse order.
In that case the part of the container we have already examined is
shifted and presents no problem.

Wouldn't you agree?
No, still too many assignments and redundant checks when an element is
removed. Better would be:

void clean( string& s )
{
string::size_type insert = 0;
for ( string::size_type index = 0; index != s.size(); ++index )
if ( isalpha( s[index] ) )
s[insert++] = tolower( s[index] );
s.erase( insert, string::npos );
}

That way, each character is only moved once. (strictly speaking that
last line could be "s.erase( insert );" but that doesn't make it clear
that all elements from insert to the end of the string are being erased.)


I don't believe I follow you - what redundant checks and assignments?


Sorry, I was wrong about the redundant checks. You followed me on the
redundant assignments though (see below.)
void clean( string& s )
{
for ( string::size_type index = s.size()-1; index >= 0; --index ) {
if ( isalpha( s[index] ) )
s[index] = tolower( s[index] );
else
s.erase( index, 1 );
}
}

Of course, the code with single erase is certainly faster when multiple
erasures are needed, though it includes an additional post-increment
(which itself expands into an assignment and increment). However, if the
container stores objects that need destruction (like classes), the code
with single erase is not useful (overwriting in-place would leave
dangling objects).


True, the code I posted assumes value semantics, but that's OK because
we are dealing with a string, not an arbitrary container.

--
Bene disserere est finis logices. -- Aristotle
Jun 3 '06 #12

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

18
by: muser | last post by:
is string converted to its integer equivalent by minusing it by 48? the function is suppose to check the fifth digit of struct member using the formula contained within the function. The function...
7
by: Forecast | last post by:
I run the following code in UNIX compiled by g++ 3.3.2 successfully. : // proj2.cc: returns a dynamic vector and prints out at main~~ : // : #include <iostream> : #include <vector> : : using...
6
by: lenny | last post by:
Hi, I've been trying to use a Sub or Function in VBA to connect to a database, make a query and return the recordset that results from the query. The connection to the database and the query...
18
by: Ian Stanley | last post by:
Hi, Continuing my strcat segmentation fault posting- I have a problem which occurs when appending two sting literals using strcat. I have tried to fix it by writing my own function that does the...
12
by: Jeff S | last post by:
In a VB.NET code behind module, I build a string for a link that points to a JavaScript function. The two lines of code below show what is relevant. PopupLink = "javascript:PopUpWindow(" &...
7
by: Ankit Aneja | last post by:
I put the code for url rewrite in my Application_BeginRequest on global.ascx some .aspx pages are in root ,some in folder named admin and some in folder named user aspx pages which are in user...
4
by: David Scemama | last post by:
Hi, I'm trying to read a database file written from a turbo Pascal program. I've set a structure to map the records in the file, but I have problem reading the file when I use VBFixedArray in...
16
by: Dany | last post by:
Our web service was working fine until we installed .net Framework 1.1 service pack 1. Uninstalling SP1 is not an option because our largest customer says service packs marked as "critical" by...
5
by: Stacey Levine | last post by:
I have a webservice that I wanted to return an ArrayList..Well the service compiles and runs when I have the output defined as ArrayList, but the WSDL defines the output as an Object so I was...
8
by: Rinaldo | last post by:
Hi, When I start my program in the debugger, there is no problem, but when not I get an exception. It appears in: private void Upload(string filename, string FTnaam) { MessageBox.Show("in...
0
by: DolphinDB | last post by:
Tired of spending countless mintues downsampling your data? Look no further! In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
0
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
1
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: ArrayDB | last post by:
The error message I've encountered is; ERROR:root:Error generating model response: exception: access violation writing 0x0000000000005140, which seems to be indicative of an access violation...
1
by: PapaRatzi | last post by:
Hello, I am teaching myself MS Access forms design and Visual Basic. I've created a table to capture a list of Top 30 singles and forms to capture new entries. The final step is a form (unbound)...
1
by: CloudSolutions | last post by:
Introduction: For many beginners and individual users, requiring a credit card and email registration may pose a barrier when starting to use cloud servers. However, some cloud server providers now...
0
by: af34tf | last post by:
Hi Guys, I have a domain whose name is BytesLimited.com, and I want to sell it. Does anyone know about platforms that allow me to list my domain in auction for free. Thank you
0
by: Faith0G | last post by:
I am starting a new it consulting business and it's been a while since I setup a new website. Is wordpress still the best web based software for hosting a 5 page website? The webpages will be...
0
isladogs
by: isladogs | last post by:
The next Access Europe User Group meeting will be on Wednesday 3 Apr 2024 starting at 18:00 UK time (6PM UTC+1) and finishing by 19:30 (7.30PM). In this session, we are pleased to welcome former...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.