|
Great, I think this is what I needed. I ma not sure about the for(int i=0;msg[i];i++)
as a terminating condition. Did you mean to put something else instead of msg[i]?
|
|
|
|
|
when msg[i] = 0, the loop will stop. ('false' = 0, in C/C++)
|
|
|
|
|
for(int i=0;msg[i]=0;i++) crashes.
for(int i=0;msg[i] !=' ';i++) seems to be okay.
|
|
|
|
|
Software2007 wrote: for(int i=0;msg[i]=0;i++) crashes.
msg[i]=0 is an assignment. (sorry, i wasn't speaking C, when i typed it, in my comment above)
'msg[i]' in a conditional is equivalent to 'msg[i]!=0'
|
|
|
|
|
Sorry, but I am a bit confused.
I am on C++ compiler now, and both of the following crash.
for(int i=0;msg[i]=0;i++) //I understand this is no good, it's an assignment
for(int i=0;msg[i]!=0;i++)//but shouldn't this work?
"Expression string out of range"
|
|
|
|
|
Software2007 wrote: but shouldn't this work?
yes, it should.
what's the value of i when it crashes ? and how long is the string?
|
|
|
|
|
I send in a string "<title>Test";
19 characters long.
The loop crashes at index 25
for(int i=0;msg[i];i++)
{
if(msg[i] == '<')
msg.replace(i,1,"<",0,4);
}
The string actually becomes 25 characters after replacing '<' with //"<", so the very last iteration, it increments i to 25, it tries to check the condition msg[25], I believe it crashes since the string is only 24 chars long ?
modified on Wednesday, September 14, 2011 4:09 PM
|
|
|
|
|
Software2007 wrote: The loop crashes at index 25
How are you calling replace_html_delimiters()?
Software2007 wrote: msg.replace(i,1,"<",0,4);
Crashing aside, why are you replacing < with the string equivalent? Wouldn't a simple msg[i] = '<'; suffice?
"One man's wage rise is another man's price increase." - Harold Wilson
"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons
"Some people are making such thorough preparation for rainy days that they aren't enjoying today's sunshine." - William Feather
|
|
|
|
|
I am trying to replace the C code in my original post. Unfortuantely, CodeProject keeps replacing my "& l t ;" with "<". I had to put spaces just now to display it, but in the code, there is no space between the 4 letters.
|
|
|
|
|
this works for me:
#include <string>
void repl(std::string &msg)
{
for(int i=0;msg[i];i++)
{
if(msg[i] == '<')
msg.replace(i,1,"<",0,4);
}
}
int _tmain(int argc, _TCHAR* argv[])
{
std::string f = "<title>blah</title>";
repl(f);
return 0;
}
|
|
|
|
|
The same code you showed me crashes at index 25! I am running just simple console app in VS2010.
It works on VS2008! Oh well!
Thanks for your help
modified on Wednesday, September 14, 2011 4:31 PM
|
|
|
|
|
how about a generic std::string find/replace fn:
void repl2(std::string &str, const char *find, const char *repl)
{
size_t findLen = strlen(find);
size_t replLen = strlen(repl);
size_t index = 0;
while (true) {
index = str.find(find, index);
if (index == std::string::npos) break;
str.replace(index, findLen, repl);
index+=replLen;
}
}
that's based on something from this thread[^].
|
|
|
|
|
It is my understanding that std::string does *NOT* include a NULL ('\0') character at the end of the string. One cannot assume a null termination.
So, basically you are using C style assumptions on C++ string objects. The way to deal with std::string is through the member functions string.length(), string.replace(), etc.
The examples others have shown you work because they stay within the object's definition of operative functions.
There is a string.c_str() member function that returns a pointer to a C style null terminated char * (http://www.cplusplus.com/reference/string/string/c_str/[^]) but that too cannot be modified by the receiving program.
If you're going to convert from C to C++, you should go all the way and avoid those old char * uses and move to some string class, either std::string or MFC/ATL CString, depending on your project's needs.
|
|
|
|
|
for (int i=0; msg[i]; i++) is equivalent to:
for (int i=0; msg[i]!=0; i++)
The end condition is the character at msg[i] being the null at the end of the string.
The compiler didn't like msg[i]=0 because that's an assignment, not an equality test. Need another '=' in there.
|
|
|
|
|
Well, I'm not even sure the straight C code works.
'msg' is a 'char *', meaning it points to a string of characters owned by the caller of the function. Presumably that string is in some buffer allocated by the caller.
When you complete the replace using the temporary 'z_buf' buffer (presumably large enough, but assume it is), you them move the new (possibly longer) string back into the buffer pointed to by 'msg'.
How do you know you aren't overwriting that buffer? How do you know it's large enough to receive the replaced string? I'm just saying, you could be clobbering something important.
|
|
|
|
|
In the C++ example, just what does
strcpy(msg,z_buf); do to the string object? Assuming it compiles, are you clobbering the object?
I don't use the std:: classes much, but if this were MFC/ATL CString, the strcpy argument would use a typecasting to get a pointer to the internal buffer of CString (char *) which you are *NEVER ALLOWED TO OVERWRITE*. Yet the strcpy() call does exactly that.
I would imagine that std::string has a similar internal structure and would be very upset if you started overwriting its content.
|
|
|
|
|
The reason there's no upper limit on "i" in either of the for loops is that this would take more code - it would require a strlen be performed once before the loop in addition to checking to see if i is equal to this length. It's less clear to read and more prone to induce error during maintenance, but I believe it to be for this reason that the way the loop is exited with a break.
Not sure why you'd go the trouble of #defining NUL as 0x0.. It would be clearer if the already provided NULL was used (less code too, since there's only 4 references to 'NUL' - 4 cases of simply adding another 'L'. In any case, the executable code is identical - it is just the source-code that suffers from reduced readability, unlike the loop-terminating-condition check, which produces a smaller executable when done this way than the more readable alternative of checking the strlen first then using a terminarting condition of i<strlen(
can't see="" any="" reason="" for="" this="" to="" crash="" when="" executing="" the="" 'msg[i]="NUL'" statement,="" unless="" you're="" trying="" pass="" a="" string="" constant,="" rather="" than="" dynamically="" loaded="" created="" string.="" example,="" i="" can="" failing
<pre="">char *htmlStr = "<html>";
replace_html_delimiters(htmlStr);
while I can see this succeeding
char *htmlStr = "<html>";
char htmlStrCopy = strdup(htmlStr);
replace_html_delimiters(htmlStrCopy);
..
.. other actions on htmlStrCopy
..
free(htmlStrCopy);
Add to that the fact that the "char z_buf[4095]" statement isn't terminated with a ';' in either case and you have rather a problem, considering that the C function called with a string containing "" returns the same string. Studying the function, it appears to scan through a string, quitting upon end of string (0x0), while perplexingly, when it intercepts a '<' character it copies all of the text except for this character, then it appends the '<' explicitly. It's 5am here, and I can't think of a circumstance that the output sring would be different to the input string.
|
|
|
|
|
You're right in that the code looks odd but I think it was because the Code Project Editor messed it up. The OP was trying to replace the < character with the sequence ampersand-l-t, a sequence which if typed into this editor will yield a <, making the code look wrong. He's really making the string bigger with the replacements.
|
|
|
|
|
Software2007 wrote: debugger would crash at msg[i] = nul in the c-like code
In that case msg is not a NULL terminated string.
How big do you expect msg to be, do you knwow?
If you do, you can add an additional check for not exceeding that length thus:
for(i=0; ; i++)
{
if(msg[i]== NUL)
break;
if(i > maxvmsglen)
break;
...
The other likelyhood is that the code:
msg[i] = NUL;
strcpy(z_buf,msg);
strcat(z_buf,"<");
strcat(z_buf,msg+i+1);
strcpy(msg,z_buf);
is mashing up the msg buffer and overflowing it. I have rarely seen such a horrible piece of code, what is it supposed to be doing?
==============================
Nothing to say.
|
|
|
|
|
This works:
string msg = "<HEAD>strange</HEAD><body>beautiful</body>";
string::size_type index;
while ((index = msg.find('<')) != string::npos)
{
msg = msg.replace(index, 1, "<");
}
Remember that strings are immutable, they cannot be altered in-place, so each replace call returns the modified string, which you must use on the next iteration. Similarly expressions such as msg[i] = '\0'; will cause an access violation. See here[^] for all the lowdown on STL string types.
|
|
|
|
|
Remember that strings are immutable
This is not true in standard C++. Did you think of C# or some pre-standard implementation of STL?
|
|
|
|
|
Cool Cow Orjan wrote: Did you think of C#
Yep, my brain can only handle one language at a time. 
However, the code still works.
|
|
|
|
|
If you want to replace the character '<' in a string with, "<", it could be done quite simply in C++ like this:
void replace_html_delimiters(std::string& str)
{
std::string::size_type pos = str.find("<");
while (std::string::npos != pos)
{
str.replace(pos, 1, "<");
pos = str.find("<", pos + 4);
}
}
Or if you want to cover the closing '>' as well:
void replace_html_delimiters(std::string& str)
{
std::string::size_type pos = str.find_first_of("<>");
while (std::string::npos != pos)
{
if ('<' == str[pos])
str.replace(pos, 1, "<");
else
str.replace(pos, 1, ">");
pos = str.find_first_of("<>", pos + 4);
}
}
By the way, I assume that you had < in your code example, and that CodeProject converted it to < when you posted it? This can be avoided by escaping out the leading ampersand (& is also a reserved character in HTML, like < and >) like this: &lt;.
Otherwise, your C code would simply replace the character '<' with the character '<', with lots of copying back and forth.
void replace_html_delimiters(char *msg)
{
for(i=0; ; i++)
{
if(msg[i]== NUL)
break; if(msg[i]=='<')
{
msg[i] = NUL; strcpy(z_buf,msg); strcat(z_buf,"<"); strcat(z_buf,msg+i+1); strcpy(msg,z_buf); }
}
}
And that could be rewritten very effectively like this:
void replace_html_delimiters(char *msg)
{
}
|
|
|
|
|
First, the original code should compile and work for any C++ compiler. Almost any C code should, as long as your function declarations contain the full parameter list (this was not mandatory in C, but is in C++). If the function doesn't work as intended in C++, then it didn't in C either!
If your intention is to refactor the code into something that more resembles C++ coding standard, here's a few pointers:
1. Do not use #define for constants. C++ introduced the const keyword for that purpose and AFAIK ANSI C as well. #define always introduces a risk, as it replaces text without concern for the context, and therefore might break your code in places that it was not meant to affect. It's even worse when #defines are used in headers, making the replacement global.
2. Do not use magic numbers. Magic numbers are numeric or string literals that are used within the code to define array boundaries, values passed to functions, or limits used for loops. It's almost always better to instead define a constant, using a name that explains its purpose or use. There are multiple advantages of doing this: First, if you ever need to change the value you only need to change it in one place, no matter how often you used it, and no matter whether others used it in places that you don't even know of; Second, the name of the constant explains what it is, saving people the effort to somehow divine it from the context or (nonexistent) comments; Third, constant names are often easier to remember than the literals they represent. And intelligent editors will even remember these names for you.
3. Use std::string instead of C-style 0-terminated strings. They are sometimes more awkward to use, but they're fast and generally more safe. They also manage their own memory, so you don't need to allocate an arbitrarily sized buffer yourself, nor do you need to care about its deallocation. Also there are already plenty of functions available in the STL, either as member functions of std::string, or as generic functions found in algorithm:: (unfortunately though, none of them exactly reciprokes your function)
4. Be careful when using index values for std::string, or in fact any of the containers of the STL. For one, many functions in the STL require iterators, not index values; most of the time index values - if provided for a container - can be used to read (or write) an element, but nothing else! Second, checking for the end (or start) of a range is often not easily possible with index values, as you have noticed yourself. The usual method for iterating oover the contents of an STL container, and that includes strings, is by using iterators.
5. Don't use break, nor multiple return statements in a function: they are just as bad as goto in that they obscure the program flow and make it unneccesarily hard to understand the code. The best way to replace a break is putting the condition into the loop header. If some of the code needs to be skipped, store the condition in a variable that is used both in the loop header and in an if statement that encapsulates the part of the loop that needs to be skipped. The important part is that the condition(s) that end a loop should always be visible in the header, so anyone trying to analyze the program flow can immediately see whether a loop will be invoked at all, and for how long. (you already rightfully asked why there is no end condition in the C-style for loop!)
6. If you need a function that works on a STL object (in this case std::string), check for functions already in existance and learn how to use them. E. g. for std::string, check out http://www.cplusplus.com/reference/string/string/[^]
With all of the above (and more) in mind, your code should like something like this (not tested):
#include <string>
#include <iostream>
const std::string LESSER_THAN_SIGN = "<"; const std::string LESSER_THAN_HTML = "<"; const std::string GREATER_THAN_SIGN = ">"; const std::string GREATER_THAN_HTML = ">";
void replace_all(std::string& text, const std::string& from, const std::string& to) {
std::size_t textpos = 0; std::size_t replaced_length = from.length(); while ((textpos = string.find(from, textpos)) != std::string::npos) {
text.replace(textpos, replaced_length, to);
}
}
int main() {
std::string text("<b>some fat text</b>");
replace_all(text, LESSER_THAN_SIGN, LESSER_THAN_HTML);
replace_all(text, GREATER_THAN_SIGN, GREATER_THAN_HTML);
std::cout << text << std::endl;
return 0;
}
Note: I used the JavaDoc style for commenting the function. This style is also used by Doxygen, a tool that like JavaDoc can automatically extract a full function level HTML documentation from these type of formatted comments. Even if you don't use DoxyGen, it is generally a good idea to define *some* standard for commenting each function.
Sorry this turned out rather longer than intended, but I hope you will appreciate it anyway 
|
|
|
|
|
Stefan_Lang wrote: Sorry this turned out rather longer than
Don't apologise, it's an excellent analysis of the issues, and solution.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
